| Message ID | 20180319165017.31908-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 19, 2018 at 09:17:16PM -0000, Patchwork wrote: > == Series Details == > > Series: drm/i915: Promote .format_mod_supported() to the lead role > URL : https://patchwork.freedesktop.org/series/40207/ > State : warning > > == Summary == > > ---- Possible new issues: > > Test kms_vblank: > Subgroup pipe-a-ts-continuation-suspend: > pass -> SKIP (shard-snb) Test requirement not met in function suspend_via_rtcwake, file ../lib/igt_aux.c:803: Test requirement: ret == 0 rtcwake test failed with 1 This failure could mean that something is wrong with the rtcwake tool or how your distro is set up. Last errno: 25, Inappropriate ioctl for device > > ---- Known issues: > > Test kms_flip: > Subgroup dpms-vs-vblank-race: > pass -> FAIL (shard-hsw) fdo#103060 > Subgroup flip-vs-absolute-wf_vblank-interruptible: > fail -> PASS (shard-hsw) fdo#100368 +1 > Test kms_setmode: > Subgroup basic: > pass -> FAIL (shard-hsw) fdo#99912 > > fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060 > fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 > fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 > > shard-apl total:3478 pass:1814 dwarn:1 dfail:0 fail:7 skip:1655 time:13028s > shard-hsw total:3478 pass:1766 dwarn:1 dfail:0 fail:3 skip:1707 time:11744s > shard-snb total:3478 pass:1357 dwarn:1 dfail:0 fail:2 skip:2118 time:7230s > Blacklisted hosts: > shard-kbl total:3478 pass:1939 dwarn:1 dfail:0 fail:9 skip:1529 time:9804s > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8399/shards.html
Ville Syrjala <ville.syrjala@linux.intel.com> writes: > From: Ville Syrjälä <ville.syrjala@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@anholt.net> > References: https://lists.freedesktop.org/archives/dri-devel/2018-March/169782.html > Signed-off-by: Ville Syrjälä <ville.syrjala@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
On Fri, Mar 30, 2018 at 12:06:11PM -0700, Eric Anholt wrote: > Ville Syrjala <ville.syrjala@linux.intel.com> writes: > > > From: Ville Syrjälä <ville.syrjala@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@anholt.net> > > References: https://lists.freedesktop.org/archives/dri-devel/2018-March/169782.html > > Signed-off-by: Ville Syrjälä <ville.syrjala@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
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; + } + 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; + } + 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 */ + 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; + } + 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; + } + 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; + } + 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; } } -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; } 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,