Re: [PATCH 1/8] drm/i915: Implement .get_format_info() hook for CCS

From: Jason Ekstrand <jason@jlekstrand.net>
To: Vidya Srinivas <vidya.srinivas@intel.com>
Cc: Ben Widawsky <ben@bwidawsk.net>,
	Intel GFX <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/8] drm/i915: Implement .get_format_info() hook for CCS
Date: Fri, 14 Jul 2017 16:16:07 -0700	[thread overview]
Message-ID: <CAOFGe95FQpCG7V1TuG0j+XhRXSLLVJ5wnBgJ2Q5giRNPR2o+3Q@mail.gmail.com> (raw)
In-Reply-To: <1499669605-3060-2-git-send-email-vidya.srinivas@intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 6374 bytes --]

FYI:  The latest version of the CCS modifier patches for mesa are now
reviewed:

https://patchwork.freedesktop.org/series/27213/

I'll be landing the prep-work patches soon.  The final couple of patches
are just waiting on the I915_FORMAT_MOD_Y_TILED_CCS in drm_fourcc.h to land
in the kernel.

On Sun, Jul 9, 2017 at 11:53 PM, Vidya Srinivas <vidya.srinivas@intel.com>
wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> SKL+ display engine can scan out certain kinds of compressed surfaces
> produced by the render engine. This involved telling the display engine
> the location of the color control surfae (CCS) which describes which
> parts of the main surface are compressed and which are not. The location
> of CCS is provided by userspace as just another plane with its own offset.
>
> By providing our own format information for the CCS formats, we should
> be able to make framebuffer_check() do the right thing for the CCS
> surface as well.
>
> Note that we'll return the same format info for both Y and Yf tiled
> format as that's what happens with the non-CCS Y vs. Yf as well. If
> desired, we could potentially return a unique pointer for each
> pixel_format+tiling+ccs combination, in which case we immediately be
> able to tell if any of that stuff changed by just comparing the
> pointers. But that does sound a bit wasteful space wise.
>
> v2: Drop the 'dev' argument from the hook
>         v3: Include the description of the CCS surface layout
> v4: Pretend CCS tiles are regular 128 byte wide Y tiles (Jason)
>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net> (v3)
> Signed-off-by: Ville Syrjä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c         |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++
> ++++++
>  include/drm/drm_mode_config.h        |  3 ++-
>  include/uapi/drm/drm_fourcc.h        |  3 +++
>  4 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 9c0152d..50da618 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -222,7 +222,7 @@ const struct drm_format_info *drm_format_info(u32
> format)
>         const struct drm_format_info *info = NULL;
>
>         if (dev->mode_config.funcs->get_format_info)
> -               info = dev->mode_config.funcs->get_format_info(mode_cmd);
> +               info = dev->mode_config.funcs->get_format_info(dev,
> mode_cmd);
>
>         if (!info)
>                 info = drm_format_info(mode_cmd->pixel_format);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 2144adc..9fb2f37 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2433,6 +2433,42 @@ static unsigned int intel_fb_modifier_to_tiling(uint64_t
> fb_modifier)
>         }
>  }
>
> +static const struct drm_format_info ccs_formats[] = {
> +       { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2,
> .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> +       { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2,
> .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> +       { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 2,
> .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> +       { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2,
> .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> +};
> +
> +static const struct drm_format_info *
> +lookup_format_info(const struct drm_format_info formats[],
> +                  int num_formats, u32 format)
> +{
> +       int i;
> +
> +       for (i = 0; i < num_formats; i++) {
> +               if (formats[i].format == format)
> +                       return &formats[i];
> +       }
> +
> +       return NULL;
> +}
> +
> +static const struct drm_format_info *
> +intel_get_format_info(struct drm_device *dev,
> +                     const struct drm_mode_fb_cmd2 *cmd)
> +{
> +       switch (cmd->modifier[0]) {
> +       case I915_FORMAT_MOD_Y_TILED_CCS:
> +       case I915_FORMAT_MOD_Yf_TILED_CCS:
> +               return lookup_format_info(ccs_formats,
> +                                         ARRAY_SIZE(ccs_formats),
> +                                         cmd->pixel_format);
> +       default:
> +               return NULL;
> +       }
> +}
> +
>  static int
>  intel_fill_fb_info(struct drm_i915_private *dev_priv,
>                    struct drm_framebuffer *fb)
> @@ -14622,6 +14658,7 @@ static void intel_atomic_state_free(struct
> drm_atomic_state *state)
>
>  static const struct drm_mode_config_funcs intel_mode_funcs = {
>         .fb_create = intel_user_framebuffer_create,
> +       .get_format_info = intel_get_format_info,
>         .output_poll_changed = intel_fbdev_output_poll_changed,
>         .atomic_check = intel_atomic_check,
>         .atomic_commit = intel_atomic_commit,
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 4298171..f0d3d38 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -81,7 +81,8 @@ struct drm_mode_config_funcs {
>          * The format information specific to the given fb metadata, or
>          * NULL if none is found.
>          */
> -       const struct drm_format_info *(*get_format_info)(const struct
> drm_mode_fb_cmd2 *mode_cmd);
> +       const struct drm_format_info *(*get_format_info)(struct drm_device
> *dev,
> +               const struct drm_mode_fb_cmd2 *mode_cmd);
>
>         /**
>          * @output_poll_changed:
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 7586c46..ee59109 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -252,6 +252,9 @@
>   */
>  #define I915_FORMAT_MOD_Yf_TILED fourcc_mod_code(INTEL, 3)
>
> +#define I915_FORMAT_MOD_Y_TILED_CCS    fourcc_mod_code(INTEL, 4)
> +#define I915_FORMAT_MOD_Yf_TILED_CCS   fourcc_mod_code(INTEL, 5)
> +
>  /*
>   * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
>   *
> --
> 1.9.1
>
>

[-- Attachment #1.2: Type: text/html, Size: 7810 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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