* [PATCH] drm/i915: Promote .format_mod_supported() to the lead role
@ 2018-03-19 16:50 Ville Syrjala
2018-03-19 18:16 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (7 more replies)
0 siblings, 8 replies; 15+ messages in thread
From: Ville Syrjala @ 2018-03-19 16:50 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
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.
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;
+ }
+
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,
--
2.16.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Promote .format_mod_supported() to the lead role
2018-03-19 16:50 [PATCH] drm/i915: Promote .format_mod_supported() to the lead role Ville Syrjala
@ 2018-03-19 18:16 ` Patchwork
2018-03-19 18:31 ` ✓ Fi.CI.BAT: success " Patchwork
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-03-19 18:16 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Promote .format_mod_supported() to the lead role
URL : https://patchwork.freedesktop.org/series/40207/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
bd9ae7d6edf8 drm/i915: Promote .format_mod_supported() to the lead role
-:19: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#19:
References: https://lists.freedesktop.org/archives/dri-devel/2018-March/169782.html
total: 0 errors, 1 warnings, 0 checks, 552 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Promote .format_mod_supported() to the lead role
2018-03-19 16:50 [PATCH] drm/i915: Promote .format_mod_supported() to the lead role Ville Syrjala
2018-03-19 18:16 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-03-19 18:31 ` Patchwork
2018-03-19 21:17 ` ✗ Fi.CI.IGT: warning " Patchwork
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-03-19 18:31 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Promote .format_mod_supported() to the lead role
URL : https://patchwork.freedesktop.org/series/40207/
State : success
== Summary ==
Series 40207v1 drm/i915: Promote .format_mod_supported() to the lead role
https://patchwork.freedesktop.org/api/1.0/series/40207/revisions/1/mbox/
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:430s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:441s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:381s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:540s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:297s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:514s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:513s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:502s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:422s
fi-cfl-s2 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:577s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:518s
fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:519s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:430s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:317s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:536s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:407s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:425s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:472s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:429s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:475s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:466s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:519s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:648s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:437s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:548s
fi-skl-6700hq total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:544s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:501s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:506s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:425s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:449s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:591s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:402s
260af42eeff094e4768265a6ec8bbcb29b87e9a0 drm-tip: 2018y-03m-19d-17h-15m-08s UTC integration manifest
bd9ae7d6edf8 drm/i915: Promote .format_mod_supported() to the lead role
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8399/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✗ Fi.CI.IGT: warning for drm/i915: Promote .format_mod_supported() to the lead role
2018-03-19 16:50 [PATCH] drm/i915: Promote .format_mod_supported() to the lead role Ville Syrjala
2018-03-19 18:16 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-03-19 18:31 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-03-19 21:17 ` Patchwork
2018-03-19 21:23 ` Ville Syrjälä
2018-03-30 19:06 ` [PATCH] " Eric Anholt
` (4 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Patchwork @ 2018-03-19 21:17 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== 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)
---- 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ✗ Fi.CI.IGT: warning for drm/i915: Promote .format_mod_supported() to the lead role
2018-03-19 21:17 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2018-03-19 21:23 ` Ville Syrjälä
0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2018-03-19 21:23 UTC (permalink / raw)
To: intel-gfx
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 Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm/i915: Promote .format_mod_supported() to the lead role
2018-03-19 16:50 [PATCH] drm/i915: Promote .format_mod_supported() to the lead role Ville Syrjala
` (2 preceding siblings ...)
2018-03-19 21:17 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2018-03-30 19:06 ` Eric Anholt
2018-04-27 16:33 ` Ville Syrjälä
2018-05-18 16:21 ` [PATCH v2] " Ville Syrjala
` (3 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Eric Anholt @ 2018-03-30 19:06 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx; +Cc: dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 24066 bytes --]
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
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm/i915: Promote .format_mod_supported() to the lead role
2018-03-30 19:06 ` [PATCH] " Eric Anholt
@ 2018-04-27 16:33 ` Ville Syrjälä
0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2018-04-27 16:33 UTC (permalink / raw)
To: Eric Anholt; +Cc: intel-gfx, dri-devel
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
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] drm/i915: Promote .format_mod_supported() to the lead role
2018-03-19 16:50 [PATCH] drm/i915: Promote .format_mod_supported() to the lead role Ville Syrjala
` (3 preceding siblings ...)
2018-03-30 19:06 ` [PATCH] " Eric Anholt
@ 2018-05-18 16:21 ` Ville Syrjala
2018-05-21 19:21 ` Eric Anholt
2018-05-18 16:33 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Promote .format_mod_supported() to the lead role (rev2) Patchwork
` (2 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjala @ 2018-05-18 16:21 UTC (permalink / raw)
To: intel-gfx
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.
v2: Rebase after NV12
Simplify
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 | 116 +++++++++++++++++++---------
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 141 ++++++++++++++++++++++++++---------
3 files changed, 186 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c9ec88acad9c..f5c078c9d0d2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13139,8 +13139,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:
@@ -13153,8 +13162,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:
@@ -13169,8 +13187,26 @@ 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:
@@ -13202,38 +13238,36 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
}
}
-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,
@@ -13241,7 +13275,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
@@ -13366,7 +13400,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,
@@ -13424,6 +13458,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
{
struct intel_plane *primary = NULL;
struct intel_plane_state *state = NULL;
+ const struct drm_plane_funcs *plane_funcs;
const uint32_t *intel_primary_formats;
unsigned int supported_rotations;
unsigned int num_formats;
@@ -13479,6 +13514,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
primary->check_plane = intel_check_primary_plane;
if (INTEL_GEN(dev_priv) >= 9) {
+ primary->has_ccs = skl_plane_has_ccs(dev_priv, pipe,
+ PLANE_PRIMARY);
+
if (skl_plane_has_planar(dev_priv, pipe, PLANE_PRIMARY)) {
intel_primary_formats = skl_pri_planar_formats;
num_formats = ARRAY_SIZE(skl_pri_planar_formats);
@@ -13487,7 +13525,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
num_formats = ARRAY_SIZE(skl_primary_formats);
}
- if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
+ if (primary->has_ccs)
modifiers = skl_format_modifiers_ccs;
else
modifiers = skl_format_modifiers_noccs;
@@ -13495,6 +13533,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
primary->update_plane = skl_update_plane;
primary->disable_plane = skl_disable_plane;
primary->get_hw_state = skl_plane_get_hw_state;
+
+ plane_funcs = &skl_plane_funcs;
} else if (INTEL_GEN(dev_priv) >= 4) {
intel_primary_formats = i965_primary_formats;
num_formats = ARRAY_SIZE(i965_primary_formats);
@@ -13503,6 +13543,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
primary->update_plane = i9xx_update_plane;
primary->disable_plane = i9xx_disable_plane;
primary->get_hw_state = i9xx_plane_get_hw_state;
+
+ plane_funcs = &i965_plane_funcs;
} else {
intel_primary_formats = i8xx_primary_formats;
num_formats = ARRAY_SIZE(i8xx_primary_formats);
@@ -13511,25 +13553,27 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
primary->update_plane = i9xx_update_plane;
primary->disable_plane = i9xx_disable_plane;
primary->get_hw_state = i9xx_plane_get_hw_state;
+
+ plane_funcs = &i8xx_plane_funcs;
}
if (INTEL_GEN(dev_priv) >= 9)
ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
- 0, &intel_plane_funcs,
+ 0, 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, 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, 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 22af249393a4..42a59b7fd736 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -954,6 +954,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 ee23613f9fd4..214cc730642c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1211,8 +1211,17 @@ 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:
@@ -1228,8 +1237,17 @@ static bool g4x_mod_supported(uint32_t format, uint64_t modifier)
}
}
-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:
@@ -1246,8 +1264,17 @@ static bool snb_mod_supported(uint32_t format, uint64_t modifier)
}
}
-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:
@@ -1269,8 +1296,26 @@ static bool vlv_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:
@@ -1302,38 +1347,48 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
}
}
-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,
@@ -1359,6 +1414,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;
@@ -1383,6 +1439,9 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
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;
intel_plane->get_hw_state = skl_plane_get_hw_state;
@@ -1396,10 +1455,12 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
num_plane_formats = ARRAY_SIZE(skl_plane_formats);
}
- if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
+ if (intel_plane->has_ccs)
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;
@@ -1411,6 +1472,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;
@@ -1427,6 +1490,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;
@@ -1439,9 +1504,13 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
if (IS_GEN6(dev_priv)) {
plane_formats = snb_plane_formats;
num_plane_formats = ARRAY_SIZE(snb_plane_formats);
+
+ plane_funcs = &snb_sprite_funcs;
} else {
plane_formats = g4x_plane_formats;
num_plane_formats = ARRAY_SIZE(g4x_plane_formats);
+
+ plane_funcs = &g4x_sprite_funcs;
}
}
@@ -1468,14 +1537,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Promote .format_mod_supported() to the lead role (rev2)
2018-03-19 16:50 [PATCH] drm/i915: Promote .format_mod_supported() to the lead role Ville Syrjala
` (4 preceding siblings ...)
2018-05-18 16:21 ` [PATCH v2] " Ville Syrjala
@ 2018-05-18 16:33 ` Patchwork
2018-05-18 16:48 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-19 0:49 ` ✓ Fi.CI.IGT: " Patchwork
7 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-05-18 16:33 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Promote .format_mod_supported() to the lead role (rev2)
URL : https://patchwork.freedesktop.org/series/40207/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
42558aaba961 drm/i915: Promote .format_mod_supported() to the lead role
-:22: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#22:
References: https://lists.freedesktop.org/archives/dri-devel/2018-March/169782.html
total: 0 errors, 1 warnings, 0 checks, 451 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Promote .format_mod_supported() to the lead role (rev2)
2018-03-19 16:50 [PATCH] drm/i915: Promote .format_mod_supported() to the lead role Ville Syrjala
` (5 preceding siblings ...)
2018-05-18 16:33 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Promote .format_mod_supported() to the lead role (rev2) Patchwork
@ 2018-05-18 16:48 ` Patchwork
2018-05-19 0:49 ` ✓ Fi.CI.IGT: " Patchwork
7 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-05-18 16:48 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Promote .format_mod_supported() to the lead role (rev2)
URL : https://patchwork.freedesktop.org/series/40207/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4209 -> Patchwork_9056 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/40207/revisions/2/mbox/
== Known issues ==
Here are the changes found in Patchwork_9056 that come from known issues:
=== IGT changes ===
==== Possible fixes ====
igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
fi-hsw-4200u: FAIL (fdo#103481) -> PASS
fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
== Participating hosts (43 -> 39) ==
Missing (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4209 -> Patchwork_9056
CI_DRM_4209: eecb2c1e793ed98c39876c92fc64cd18a7fe6412 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4487: eccae1360d6d01e73c6af2bd97122cef708207ef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9056: 42558aaba96116a3308a8f976d1dd9e64929de91 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4487: 6ab75f7eb5e1dccbb773e1739beeb2d7cbd6ad0d @ git://anongit.freedesktop.org/piglit
== Linux commits ==
42558aaba961 drm/i915: Promote .format_mod_supported() to the lead role
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9056/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Promote .format_mod_supported() to the lead role (rev2)
2018-03-19 16:50 [PATCH] drm/i915: Promote .format_mod_supported() to the lead role Ville Syrjala
` (6 preceding siblings ...)
2018-05-18 16:48 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-05-19 0:49 ` Patchwork
7 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-05-19 0:49 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Promote .format_mod_supported() to the lead role (rev2)
URL : https://patchwork.freedesktop.org/series/40207/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4209_full -> Patchwork_9056_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9056_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9056_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/40207/revisions/2/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9056_full:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_schedule@deep-bsd1:
shard-kbl: PASS -> SKIP +1
igt@gem_exec_schedule@deep-bsd2:
shard-kbl: SKIP -> PASS +2
igt@kms_plane_lowres@pipe-c-tiling-x:
shard-apl: SKIP -> PASS
== Known issues ==
Here are the changes found in Patchwork_9056_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
shard-glk: PASS -> FAIL (fdo#104873)
igt@kms_flip@dpms-vs-vblank-race:
shard-hsw: PASS -> FAIL (fdo#103060)
igt@kms_flip_tiling@flip-x-tiled:
shard-glk: PASS -> FAIL (fdo#104724, fdo#103822)
igt@kms_plane_lowres@pipe-c-tiling-x:
shard-glk: PASS -> DMESG-WARN (fdo#106247)
==== Possible fixes ====
igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
shard-glk: FAIL (fdo#105703) -> PASS
igt@kms_flip_tiling@flip-to-x-tiled:
shard-glk: FAIL (fdo#104724) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt:
shard-apl: DMESG-FAIL (fdo#105602, fdo#103558) -> PASS
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-wc:
shard-glk: FAIL (fdo#104724, fdo#103167) -> PASS +2
igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
shard-apl: DMESG-WARN (fdo#105602, fdo#103558) -> PASS +9
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
shard-kbl: FAIL (fdo#99912) -> PASS
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
fdo#106247 https://bugs.freedesktop.org/show_bug.cgi?id=106247
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (9 -> 9) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4209 -> Patchwork_9056
CI_DRM_4209: eecb2c1e793ed98c39876c92fc64cd18a7fe6412 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4487: eccae1360d6d01e73c6af2bd97122cef708207ef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9056: 42558aaba96116a3308a8f976d1dd9e64929de91 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4487: 6ab75f7eb5e1dccbb773e1739beeb2d7cbd6ad0d @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9056/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/i915: Promote .format_mod_supported() to the lead role
2018-05-18 16:21 ` [PATCH v2] " Ville Syrjala
@ 2018-05-21 19:21 ` Eric Anholt
2018-05-22 16:36 ` Ville Syrjälä
0 siblings, 1 reply; 15+ messages in thread
From: Eric Anholt @ 2018-05-21 19:21 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 913 bytes --]
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.
This seems like it would be a lot shorter if you just had a helper to
check if your format and modifier was in drm_plane->format_types and
drm_plane->modifiers, since then you wouldn't be duplicating your tables
and you wouldn't need has_ccs either.
However, it's not my driver and it unblocks vc4's patch, so:
Reviewed-by: Eric Anholt <eric@anholt.net>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/i915: Promote .format_mod_supported() to the lead role
2018-05-21 19:21 ` Eric Anholt
@ 2018-05-22 16:36 ` Ville Syrjälä
2018-05-30 18:30 ` Eric Anholt
0 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2018-05-22 16:36 UTC (permalink / raw)
To: Eric Anholt; +Cc: intel-gfx
On Mon, May 21, 2018 at 12:21:01PM -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.
>
> This seems like it would be a lot shorter if you just had a helper to
> check if your format and modifier was in drm_plane->format_types and
> drm_plane->modifiers, since then you wouldn't be duplicating your tables
> and you wouldn't need has_ccs either.
I suppose. And I guess that's where I started originally :/
But I'm not sure if it's better go that route or the other route of
reducing the arrays to some simple supersets and also utilize
.format_mod_supported() in plane init to filter out the unsupported
formats when populating the plane's format list. Probably best not
dwell on this too much for now so that we can at least make some
progress :)
>
> However, it's not my driver and it unblocks vc4's patch, so:
>
> Reviewed-by: Eric Anholt <eric@anholt.net>
Thanks. I'm guessing we should push this into drm-misc-next so
that you can pile your core/sand bits on top?
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/i915: Promote .format_mod_supported() to the lead role
2018-05-22 16:36 ` Ville Syrjälä
@ 2018-05-30 18:30 ` Eric Anholt
2018-05-30 20:10 ` Ville Syrjälä
0 siblings, 1 reply; 15+ messages in thread
From: Eric Anholt @ 2018-05-30 18:30 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 1677 bytes --]
Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
> On Mon, May 21, 2018 at 12:21:01PM -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.
>>
>> This seems like it would be a lot shorter if you just had a helper to
>> check if your format and modifier was in drm_plane->format_types and
>> drm_plane->modifiers, since then you wouldn't be duplicating your tables
>> and you wouldn't need has_ccs either.
>
> I suppose. And I guess that's where I started originally :/
>
> But I'm not sure if it's better go that route or the other route of
> reducing the arrays to some simple supersets and also utilize
> .format_mod_supported() in plane init to filter out the unsupported
> formats when populating the plane's format list. Probably best not
> dwell on this too much for now so that we can at least make some
> progress :)
>
>>
>> However, it's not my driver and it unblocks vc4's patch, so:
>>
>> Reviewed-by: Eric Anholt <eric@anholt.net>
>
> Thanks. I'm guessing we should push this into drm-misc-next so
> that you can pile your core/sand bits on top?
That would be wonderful.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/i915: Promote .format_mod_supported() to the lead role
2018-05-30 18:30 ` Eric Anholt
@ 2018-05-30 20:10 ` Ville Syrjälä
0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2018-05-30 20:10 UTC (permalink / raw)
To: Eric Anholt; +Cc: intel-gfx
On Wed, May 30, 2018 at 11:30:26AM -0700, Eric Anholt wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
>
> > On Mon, May 21, 2018 at 12:21:01PM -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.
> >>
> >> This seems like it would be a lot shorter if you just had a helper to
> >> check if your format and modifier was in drm_plane->format_types and
> >> drm_plane->modifiers, since then you wouldn't be duplicating your tables
> >> and you wouldn't need has_ccs either.
> >
> > I suppose. And I guess that's where I started originally :/
> >
> > But I'm not sure if it's better go that route or the other route of
> > reducing the arrays to some simple supersets and also utilize
> > .format_mod_supported() in plane init to filter out the unsupported
> > formats when populating the plane's format list. Probably best not
> > dwell on this too much for now so that we can at least make some
> > progress :)
> >
> >>
> >> However, it's not my driver and it unblocks vc4's patch, so:
> >>
> >> Reviewed-by: Eric Anholt <eric@anholt.net>
> >
> > Thanks. I'm guessing we should push this into drm-misc-next so
> > that you can pile your core/sand bits on top?
>
> That would be wonderful.
Now pushed. Had to resolve a few nv12 conflicts each way. I guess
someone else will get to deal with that same fun at some point
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-05-30 20:10 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-19 16:50 [PATCH] drm/i915: Promote .format_mod_supported() to the lead role Ville Syrjala
2018-03-19 18:16 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-03-19 18:31 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-19 21:17 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-03-19 21:23 ` Ville Syrjälä
2018-03-30 19:06 ` [PATCH] " Eric Anholt
2018-04-27 16:33 ` Ville Syrjälä
2018-05-18 16:21 ` [PATCH v2] " Ville Syrjala
2018-05-21 19:21 ` Eric Anholt
2018-05-22 16:36 ` Ville Syrjälä
2018-05-30 18:30 ` Eric Anholt
2018-05-30 20:10 ` Ville Syrjälä
2018-05-18 16:33 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Promote .format_mod_supported() to the lead role (rev2) Patchwork
2018-05-18 16:48 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-19 0:49 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.