* [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
* 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
* ✗ 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).