[Intel-gfx] [PATCH] drm/i915: Promote .format_mod_supported() to the lead role

Eric Anholt eric at anholt.net
Fri Mar 30 19:06:11 UTC 2018


Ville Syrjala <ville.syrjala at linux.intel.com> writes:

> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Up to now we've used the plane's modifier list as the primary
> source of information for which modifiers are supported by a
> given plane. In order to allow auxiliary metadata to be embedded
> within the bits of the modifier we need to stop doing that.
>
> Thus we have to make .format_mod_supported() aware of the plane's
> capabilities and gracefully deal with any modifier being passed
> in directly from userspace.

I took a look, and I think you have the chance to delete a whole ton of
code if you keep the assumption that the core will check that the format
is one of plane->format_types.

>
> Cc: Eric Anholt <eric at anholt.net>
> References: https://lists.freedesktop.org/archives/dri-devel/2018-March/169782.html
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 147 +++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_drv.h     |   1 +
>  drivers/gpu/drm/i915/intel_sprite.c  | 194 ++++++++++++++++++++++-------------
>  3 files changed, 210 insertions(+), 132 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3e7ab75e1b41..d717004be0b8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -88,15 +88,7 @@ static const uint32_t skl_primary_formats[] = {
>  	DRM_FORMAT_VYUY,
>  };
>  
> -static const uint64_t skl_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_format_modifiers_ccs[] = {
> +static const uint64_t skl_format_modifiers[] = {
>  	I915_FORMAT_MOD_Yf_TILED_CCS,
>  	I915_FORMAT_MOD_Y_TILED_CCS,
>  	I915_FORMAT_MOD_Yf_TILED,
> @@ -12997,8 +12989,17 @@ void intel_plane_destroy(struct drm_plane *plane)
>  	kfree(to_intel_plane(plane));
>  }
>  
> -static bool i8xx_mod_supported(uint32_t format, uint64_t modifier)
> +static bool i8xx_plane_format_mod_supported(struct drm_plane *_plane,
> +					    u32 format, u64 modifier)
>  {
> +	switch (modifier) {
> +	case DRM_FORMAT_MOD_LINEAR:
> +	case I915_FORMAT_MOD_X_TILED:
> +		break;
> +	default:
> +		return false;
> +	}
> +

I think you could just remove the format-dependent switch below in favor
of s/break/return true/.  It's just a list of all the formats in
i8xx_primary_formats[].

>  	switch (format) {
>  	case DRM_FORMAT_C8:
>  	case DRM_FORMAT_RGB565:
> @@ -13011,8 +13012,17 @@ static bool i8xx_mod_supported(uint32_t format, uint64_t modifier)
>  	}
>  }
>  
> -static bool i965_mod_supported(uint32_t format, uint64_t modifier)
> +static bool i965_plane_format_mod_supported(struct drm_plane *_plane,
> +					    u32 format, u64 modifier)
>  {
> +	switch (modifier) {
> +	case DRM_FORMAT_MOD_LINEAR:
> +	case I915_FORMAT_MOD_X_TILED:
> +		break;
> +	default:
> +		return false;
> +	}

Again, there's no format dependence, so drop the switch statement, and
probably just reuse the mod_supported func from 8xx.

> +
>  	switch (format) {
>  	case DRM_FORMAT_C8:
>  	case DRM_FORMAT_RGB565:
> @@ -13027,17 +13037,37 @@ static bool i965_mod_supported(uint32_t format, uint64_t modifier)
>  	}
>  }
>  
> -static bool skl_mod_supported(uint32_t format, uint64_t modifier)
> +static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
> +					   u32 format, u64 modifier)
>  {
> +	struct intel_plane *plane = to_intel_plane(_plane);
> +
> +	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:
> +		break;
> +	case I915_FORMAT_MOD_Y_TILED_CCS:
> +	case I915_FORMAT_MOD_Yf_TILED_CCS:
> +		if (!plane->has_ccs)
> +			return false;
> +		break;
> +	default:
> +		return false;
> +	}
> +
>  	switch (format) {
>  	case DRM_FORMAT_XRGB8888:
>  	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 */

I think these SKL changes could have just been done with a "&&
plane->has_ccs" in this '-' area.  I do think the new version is more
readable, though.

> +		return modifier == DRM_FORMAT_MOD_LINEAR ||
> +			modifier == I915_FORMAT_MOD_X_TILED ||
> +			modifier == I915_FORMAT_MOD_Y_TILED ||
> +			modifier == I915_FORMAT_MOD_Yf_TILED ||
> +			modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> +			modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
>  	case DRM_FORMAT_RGB565:
>  	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_XBGR2101010:
> @@ -13045,52 +13075,49 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
>  	case DRM_FORMAT_YVYU:
>  	case DRM_FORMAT_UYVY:
>  	case DRM_FORMAT_VYUY:
> -		if (modifier == I915_FORMAT_MOD_Yf_TILED)
> -			return true;
> -		/* fall through */
> +		return modifier == DRM_FORMAT_MOD_LINEAR ||
> +			modifier == I915_FORMAT_MOD_X_TILED ||
> +			modifier == I915_FORMAT_MOD_Y_TILED ||
> +			modifier == I915_FORMAT_MOD_Yf_TILED;
>  	case DRM_FORMAT_C8:
> -		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> -		    modifier == I915_FORMAT_MOD_X_TILED ||
> -		    modifier == I915_FORMAT_MOD_Y_TILED)
> -			return true;
> -		/* fall through */
> +		return modifier == DRM_FORMAT_MOD_LINEAR ||
> +			modifier == I915_FORMAT_MOD_X_TILED ||
> +			modifier == I915_FORMAT_MOD_Y_TILED;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool intel_primary_plane_format_mod_supported(struct drm_plane *plane,
> -						     uint32_t format,
> -						     uint64_t modifier)
> +static bool intel_cursor_format_mod_supported(struct drm_plane *_plane,
> +					      u32 format, u64 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 &&
> -	    modifier != DRM_FORMAT_MOD_LINEAR)
> -		return false;
> -
> -	if (INTEL_GEN(dev_priv) >= 9)
> -		return skl_mod_supported(format, modifier);
> -	else if (INTEL_GEN(dev_priv) >= 4)
> -		return i965_mod_supported(format, modifier);
> -	else
> -		return i8xx_mod_supported(format, modifier);
> +	return modifier == DRM_FORMAT_MOD_LINEAR &&
> +		format == DRM_FORMAT_ARGB8888;
>  }
>  
> -static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
> -						    uint32_t format,
> -						    uint64_t modifier)
> -{
> -	if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
> -		return false;
> +static struct drm_plane_funcs skl_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = intel_plane_destroy,
> +	.atomic_get_property = intel_plane_atomic_get_property,
> +	.atomic_set_property = intel_plane_atomic_set_property,
> +	.atomic_duplicate_state = intel_plane_duplicate_state,
> +	.atomic_destroy_state = intel_plane_destroy_state,
> +	.format_mod_supported = skl_plane_format_mod_supported,
> +};
>  
> -	return modifier == DRM_FORMAT_MOD_LINEAR && format == DRM_FORMAT_ARGB8888;
> -}
> +static struct drm_plane_funcs i965_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = intel_plane_destroy,
> +	.atomic_get_property = intel_plane_atomic_get_property,
> +	.atomic_set_property = intel_plane_atomic_set_property,
> +	.atomic_duplicate_state = intel_plane_duplicate_state,
> +	.atomic_destroy_state = intel_plane_destroy_state,
> +	.format_mod_supported = i965_plane_format_mod_supported,
> +};
>  
> -static struct drm_plane_funcs intel_plane_funcs = {
> +static struct drm_plane_funcs i8xx_plane_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
>  	.destroy = intel_plane_destroy,
> @@ -13098,7 +13125,7 @@ static struct drm_plane_funcs intel_plane_funcs = {
>  	.atomic_set_property = intel_plane_atomic_set_property,
>  	.atomic_duplicate_state = intel_plane_duplicate_state,
>  	.atomic_destroy_state = intel_plane_destroy_state,
> -	.format_mod_supported = intel_primary_plane_format_mod_supported,
> +	.format_mod_supported = i8xx_plane_format_mod_supported,
>  };
>  
>  static int
> @@ -13223,7 +13250,7 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
>  	.atomic_set_property = intel_plane_atomic_set_property,
>  	.atomic_duplicate_state = intel_plane_duplicate_state,
>  	.atomic_destroy_state = intel_plane_destroy_state,
> -	.format_mod_supported = intel_cursor_plane_format_mod_supported,
> +	.format_mod_supported = intel_cursor_format_mod_supported,
>  };
>  
>  static bool i9xx_plane_has_fbc(struct drm_i915_private *dev_priv,
> @@ -13314,11 +13341,10 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		intel_primary_formats = skl_primary_formats;
>  		num_formats = ARRAY_SIZE(skl_primary_formats);
> +		modifiers = skl_format_modifiers;
>  
> -		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
> -			modifiers = skl_format_modifiers_ccs;
> -		else
> -			modifiers = skl_format_modifiers_noccs;
> +		primary->has_ccs = skl_plane_has_ccs(dev_priv, pipe,
> +						     PLANE_PRIMARY);
>  
>  		primary->update_plane = skl_update_plane;
>  		primary->disable_plane = skl_disable_plane;
> @@ -13343,21 +13369,22 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  
>  	if (INTEL_GEN(dev_priv) >= 9)
>  		ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
> -					       0, &intel_plane_funcs,
> +					       0, &skl_plane_funcs,
>  					       intel_primary_formats, num_formats,
>  					       modifiers,
>  					       DRM_PLANE_TYPE_PRIMARY,
>  					       "plane 1%c", pipe_name(pipe));
>  	else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv))
>  		ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
> -					       0, &intel_plane_funcs,
> +					       0, &i965_plane_funcs,
>  					       intel_primary_formats, num_formats,
>  					       modifiers,
>  					       DRM_PLANE_TYPE_PRIMARY,
>  					       "primary %c", pipe_name(pipe));
>  	else
>  		ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
> -					       0, &intel_plane_funcs,
> +					       0, IS_GEN4(dev_priv) ?
> +					       &i965_plane_funcs : &i8xx_plane_funcs,
>  					       intel_primary_formats, num_formats,
>  					       modifiers,
>  					       DRM_PLANE_TYPE_PRIMARY,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a215aa78b0be..e0b70c563e9f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -937,6 +937,7 @@ struct intel_plane {
>  	enum pipe pipe;
>  	bool can_scale;
>  	bool has_fbc;
> +	bool has_ccs;
>  	int max_downscale;
>  	uint32_t frontbuffer_bit;
>  
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index dbdcf85032df..f4dbdd6d1ddc 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1248,15 +1248,7 @@ static uint32_t skl_plane_formats[] = {
>  	DRM_FORMAT_VYUY,
>  };
>  
> -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[] = {
> +static const uint64_t skl_plane_format_modifiers[] = {
>  	I915_FORMAT_MOD_Yf_TILED_CCS,
>  	I915_FORMAT_MOD_Y_TILED_CCS,
>  	I915_FORMAT_MOD_Yf_TILED,
> @@ -1266,25 +1258,41 @@ static const uint64_t skl_plane_format_modifiers_ccs[] = {
>  	DRM_FORMAT_MOD_INVALID
>  };
>  
> -static bool g4x_mod_supported(uint32_t format, uint64_t modifier)
> +static bool g4x_sprite_format_mod_supported(struct drm_plane *_plane,
> +					    u32 format, u64 modifier)
>  {
> +	switch (modifier) {
> +	case DRM_FORMAT_MOD_LINEAR:
> +	case I915_FORMAT_MOD_X_TILED:
> +		break;
> +	default:
> +		return false;
> +	}
> +

Reuse the same LINEAR/MOD_X_TILED func from intel_display.c?

>  	switch (format) {
>  	case DRM_FORMAT_XRGB8888:
>  	case DRM_FORMAT_YUYV:
>  	case DRM_FORMAT_YVYU:
>  	case DRM_FORMAT_UYVY:
>  	case DRM_FORMAT_VYUY:
> -		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> -		    modifier == I915_FORMAT_MOD_X_TILED)
> -			return true;
> -		/* fall through */
> +		return modifier == DRM_FORMAT_MOD_LINEAR ||
> +			modifier == I915_FORMAT_MOD_X_TILED;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool snb_mod_supported(uint32_t format, uint64_t modifier)
> +static bool snb_sprite_format_mod_supported(struct drm_plane *_plane,
> +					    u32 format, u64 modifier)
>  {
> +	switch (modifier) {
> +	case DRM_FORMAT_MOD_LINEAR:
> +	case I915_FORMAT_MOD_X_TILED:
> +		break;
> +	default:
> +		return false;
> +	}

Reuse the same LINEAR/MOD_X_TILED func from intel_display.c?

> +
>  	switch (format) {
>  	case DRM_FORMAT_XRGB8888:
>  	case DRM_FORMAT_XBGR8888:
> @@ -1292,17 +1300,24 @@ static bool snb_mod_supported(uint32_t format, uint64_t modifier)
>  	case DRM_FORMAT_YVYU:
>  	case DRM_FORMAT_UYVY:
>  	case DRM_FORMAT_VYUY:
> -		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> -		    modifier == I915_FORMAT_MOD_X_TILED)
> -			return true;
> -		/* fall through */
> +		return modifier == DRM_FORMAT_MOD_LINEAR ||
> +			modifier == I915_FORMAT_MOD_X_TILED;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool vlv_mod_supported(uint32_t format, uint64_t modifier)
> +static bool vlv_sprite_format_mod_supported(struct drm_plane *_plane,
> +					    u32 format, u64 modifier)
>  {
> +	switch (modifier) {
> +	case DRM_FORMAT_MOD_LINEAR:
> +	case I915_FORMAT_MOD_X_TILED:
> +		break;
> +	default:
> +		return false;
> +	}

Reuse the same LINEAR/MOD_X_TILED func from intel_display.c?

> +
>  	switch (format) {
>  	case DRM_FORMAT_RGB565:
>  	case DRM_FORMAT_ABGR8888:
> @@ -1315,26 +1330,44 @@ static bool vlv_mod_supported(uint32_t format, uint64_t modifier)
>  	case DRM_FORMAT_YVYU:
>  	case DRM_FORMAT_UYVY:
>  	case DRM_FORMAT_VYUY:
> -		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> -		    modifier == I915_FORMAT_MOD_X_TILED)
> -			return true;
> -		/* fall through */
> +		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)
> +static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
> +					   u32 format, u64 modifier)
>  {
> +	struct intel_plane *plane = to_intel_plane(_plane);
> +
> +	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:
> +		break;
> +	case I915_FORMAT_MOD_Y_TILED_CCS:
> +	case I915_FORMAT_MOD_Yf_TILED_CCS:
> +		if (!plane->has_ccs)
> +			return false;
> +		break;
> +	default:
> +		return false;
> +	}
> +
>  	switch (format) {
>  	case DRM_FORMAT_XRGB8888:
>  	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 */
> +		return modifier == DRM_FORMAT_MOD_LINEAR ||
> +			modifier == I915_FORMAT_MOD_X_TILED ||
> +			modifier == I915_FORMAT_MOD_Y_TILED ||
> +			modifier == I915_FORMAT_MOD_Yf_TILED ||
> +			modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> +			modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
>  	case DRM_FORMAT_RGB565:
>  	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_XBGR2101010:
> @@ -1342,52 +1375,61 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
>  	case DRM_FORMAT_YVYU:
>  	case DRM_FORMAT_UYVY:
>  	case DRM_FORMAT_VYUY:
> -		if (modifier == I915_FORMAT_MOD_Yf_TILED)
> -			return true;
> -		/* fall through */
> +		return modifier == DRM_FORMAT_MOD_LINEAR ||
> +			modifier == I915_FORMAT_MOD_X_TILED ||
> +			modifier == I915_FORMAT_MOD_Y_TILED ||
> +			modifier == I915_FORMAT_MOD_Yf_TILED;
>  	case DRM_FORMAT_C8:
> -		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> -		    modifier == I915_FORMAT_MOD_X_TILED ||
> -		    modifier == I915_FORMAT_MOD_Y_TILED)
> -			return true;
> -		/* fall through */
> +		return modifier == DRM_FORMAT_MOD_LINEAR ||
> +			modifier == I915_FORMAT_MOD_X_TILED ||
> +			modifier == I915_FORMAT_MOD_Y_TILED;
>  	default:
>  		return false;
>  	}

This looks like the same skl func from intel_display.c.  Can we reuse it?

>  }
>  
> -static bool intel_sprite_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;
> +static const struct drm_plane_funcs g4x_sprite_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = intel_plane_destroy,
> +	.atomic_get_property = intel_plane_atomic_get_property,
> +	.atomic_set_property = intel_plane_atomic_set_property,
> +	.atomic_duplicate_state = intel_plane_duplicate_state,
> +	.atomic_destroy_state = intel_plane_destroy_state,
> +	.format_mod_supported = g4x_sprite_format_mod_supported,
> +};
>  
> -	if ((modifier >> 56) != DRM_FORMAT_MOD_VENDOR_INTEL &&
> -	    modifier != DRM_FORMAT_MOD_LINEAR)
> -		return false;
> +static const struct drm_plane_funcs snb_sprite_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = intel_plane_destroy,
> +	.atomic_get_property = intel_plane_atomic_get_property,
> +	.atomic_set_property = intel_plane_atomic_set_property,
> +	.atomic_duplicate_state = intel_plane_duplicate_state,
> +	.atomic_destroy_state = intel_plane_destroy_state,
> +	.format_mod_supported = snb_sprite_format_mod_supported,
> +};
>  
> -	if (INTEL_GEN(dev_priv) >= 9)
> -		return skl_mod_supported(format, modifier);
> -	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> -		return vlv_mod_supported(format, modifier);
> -	else if (INTEL_GEN(dev_priv) >= 6)
> -		return snb_mod_supported(format, modifier);
> -	else
> -		return g4x_mod_supported(format, modifier);
> -}
> +static const struct drm_plane_funcs vlv_sprite_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = intel_plane_destroy,
> +	.atomic_get_property = intel_plane_atomic_get_property,
> +	.atomic_set_property = intel_plane_atomic_set_property,
> +	.atomic_duplicate_state = intel_plane_duplicate_state,
> +	.atomic_destroy_state = intel_plane_destroy_state,
> +	.format_mod_supported = vlv_sprite_format_mod_supported,
> +};
>  
> -static const struct drm_plane_funcs intel_sprite_plane_funcs = {
> -        .update_plane = drm_atomic_helper_update_plane,
> -        .disable_plane = drm_atomic_helper_disable_plane,
> -        .destroy = intel_plane_destroy,
> -        .atomic_get_property = intel_plane_atomic_get_property,
> -        .atomic_set_property = intel_plane_atomic_set_property,
> -        .atomic_duplicate_state = intel_plane_duplicate_state,
> -        .atomic_destroy_state = intel_plane_destroy_state,
> -        .format_mod_supported = intel_sprite_plane_format_mod_supported,
> +static const struct drm_plane_funcs skl_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = intel_plane_destroy,
> +	.atomic_get_property = intel_plane_atomic_get_property,
> +	.atomic_set_property = intel_plane_atomic_set_property,
> +	.atomic_duplicate_state = intel_plane_duplicate_state,
> +	.atomic_destroy_state = intel_plane_destroy_state,
> +	.format_mod_supported = skl_plane_format_mod_supported,
>  };
>  
>  bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
> @@ -1413,6 +1455,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  {
>  	struct intel_plane *intel_plane = NULL;
>  	struct intel_plane_state *state = NULL;
> +	const struct drm_plane_funcs *plane_funcs;
>  	unsigned long possible_crtcs;
>  	const uint32_t *plane_formats;
>  	const uint64_t *modifiers;
> @@ -1436,6 +1479,8 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		intel_plane->can_scale = true;
>  		state->scaler_id = -1;
> +		intel_plane->has_ccs = skl_plane_has_ccs(dev_priv, pipe,
> +							 PLANE_SPRITE0 + plane);
>  
>  		intel_plane->update_plane = skl_update_plane;
>  		intel_plane->disable_plane = skl_disable_plane;
> @@ -1443,11 +1488,9 @@ 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;
>  
> -		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
> -			modifiers = skl_plane_format_modifiers_ccs;
> -		else
> -			modifiers = skl_plane_format_modifiers_noccs;
> +		plane_funcs = &skl_plane_funcs;
>  	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>  		intel_plane->can_scale = false;
>  		intel_plane->max_downscale = 1;
> @@ -1459,6 +1502,8 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  		plane_formats = vlv_plane_formats;
>  		num_plane_formats = ARRAY_SIZE(vlv_plane_formats);
>  		modifiers = i9xx_plane_format_modifiers;
> +
> +		plane_funcs = &vlv_sprite_funcs;
>  	} else if (INTEL_GEN(dev_priv) >= 7) {
>  		if (IS_IVYBRIDGE(dev_priv)) {
>  			intel_plane->can_scale = true;
> @@ -1475,6 +1520,8 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  		plane_formats = snb_plane_formats;
>  		num_plane_formats = ARRAY_SIZE(snb_plane_formats);
>  		modifiers = i9xx_plane_format_modifiers;
> +
> +		plane_funcs = &snb_sprite_funcs;
>  	} else {
>  		intel_plane->can_scale = true;
>  		intel_plane->max_downscale = 16;
> @@ -1491,6 +1538,9 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  			plane_formats = g4x_plane_formats;
>  			num_plane_formats = ARRAY_SIZE(g4x_plane_formats);
>  		}
> +
> +		plane_funcs = IS_GEN6(dev_priv) ?
> +			&snb_sprite_funcs : &g4x_sprite_funcs;

Move htis assignment into the IS_GEN6() above?

>  	}
>  
>  	if (INTEL_GEN(dev_priv) >= 9) {
> @@ -1516,14 +1566,14 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  
>  	if (INTEL_GEN(dev_priv) >= 9)
>  		ret = drm_universal_plane_init(&dev_priv->drm, &intel_plane->base,
> -					       possible_crtcs, &intel_sprite_plane_funcs,
> +					       possible_crtcs, plane_funcs,
>  					       plane_formats, num_plane_formats,
>  					       modifiers,
>  					       DRM_PLANE_TYPE_OVERLAY,
>  					       "plane %d%c", plane + 2, pipe_name(pipe));
>  	else
>  		ret = drm_universal_plane_init(&dev_priv->drm, &intel_plane->base,
> -					       possible_crtcs, &intel_sprite_plane_funcs,
> +					       possible_crtcs, plane_funcs,
>  					       plane_formats, num_plane_formats,
>  					       modifiers,
>  					       DRM_PLANE_TYPE_OVERLAY,
> -- 
> 2.16.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20180330/d4a776e0/attachment-0001.sig>


More information about the Intel-gfx mailing list