Re: [Intel-gfx] [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

Hi,

On 18/11/2016 19:53, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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.
>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++++
>  include/uapi/drm/drm_fourcc.h        |  3 +++
>  2 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7b7135be3b9e..de6909770c68 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2488,6 +2488,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)
> @@ -15922,6 +15958,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
>
>  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/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index a5890bf44c0a..2926d916f199 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -218,6 +218,9 @@ extern "C" {
>   */
>  #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)
> +

I think when fb modifiers were started the idea was that we would later 
partition our vendor bit space for different classes of things and have 
helper functions to extract the tiling, etc, from them.

For example have first 3-4 bits represent the tiling, then in this case 
one bit for CCS, etc.

Have you considered that when adding these ones, and concluded this 
different scheme is better for some reason?

Regards,

Tvrtko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel