[Intel-gfx] [PATCH] drm/i915: Promote .format_mod_supported() to the lead role
Ville Syrjälä
ville.syrjala at linux.intel.com
Fri Apr 27 16:33:12 UTC 2018
On Fri, Mar 30, 2018 at 12:06:11PM -0700, Eric Anholt wrote:
> 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.
I'm not particularly happy about splitting the roles that way. Makes it
harder to figure out what exactly is supported when you have to go look
at two different sources of information.
But I do agree that the duplication isn't all that nice either. I was
actually wondering whether I could just remove the format/modifier
arrays entirely and rely purely on .format_mod_supported(). But I guess
I'd have to at least keep one set of arrays to give the core something
which it could use when calling .format_mod_supported(). So I'd have to
at least keep one of each array, and just make them the superset of all
supported format/modifiers between all the platform we have in i915.
>
> >
> > 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.
Yeah, I wanted to make it more straightforward to understand.
>
> > + 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?
I have posted patches to eliminate the ugly code/data duplication for
SKL planes.
>
> > }
> >
> > -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?
Hmm. Not sure how I missed that one. Makes sense.
>
> > }
> >
> > 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
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list