* [PATCH 0/4] Intel primary plane support (v7)
@ 2014-05-29 15:06 Matt Roper
2014-05-29 15:06 ` [PATCH 1/4] drm: Check CRTC compatibility in setplane Matt Roper
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:06 UTC (permalink / raw)
To: intel-gfx
Re-posting the final versions of these patches now that they all have a r-b, as
requested by Daniel. Corresponding i-g-t tests will be sent shortly.
Previous patch review thread and comments are here:
http://lists.freedesktop.org/archives/intel-gfx/2014-April/044527.html
Matt Roper (4):
drm: Check CRTC compatibility in setplane
drm/plane-helper: Add drm_plane_helper_check_update() (v3)
drm/i915: don't force full modeset if primary plane is disabled (v2)
drm/i915: Intel-specific primary plane handling (v8)
drivers/gpu/drm/drm_crtc.c | 7 +
drivers/gpu/drm/drm_plane_helper.c | 130 +++++++++++++-----
drivers/gpu/drm/i915/intel_display.c | 255 ++++++++++++++++++++++++++++++++++-
include/drm/drm_plane_helper.h | 22 +++
4 files changed, 373 insertions(+), 41 deletions(-)
--
1.8.5.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] drm: Check CRTC compatibility in setplane
2014-05-29 15:06 [PATCH 0/4] Intel primary plane support (v7) Matt Roper
@ 2014-05-29 15:06 ` Matt Roper
2014-05-29 15:06 ` [PATCH 2/4] drm/plane-helper: Add drm_plane_helper_check_update() (v3) Matt Roper
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:06 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
The DRM core setplane code should check that the plane is usable on the
specified CRTC before calling into the driver.
Prior to this patch, a plane's possible_crtcs field was purely
informational for userspace and was never actually verified at the
kernel level (aside from the primary plane helper).
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Chon Ming Lee <chon.ming.lee@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 7 +++++++
drivers/gpu/drm/drm_plane_helper.c | 6 ------
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 37a3e07..1cfd1e3 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2153,6 +2153,13 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
}
crtc = obj_to_crtc(obj);
+ /* Check whether this plane is usable on this CRTC */
+ if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {
+ DRM_DEBUG_KMS("Invalid crtc for plane\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
if (!fb) {
DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index d966afa..b601233 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -137,12 +137,6 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
return -EINVAL;
}
- /* Primary planes are locked to their owning CRTC */
- if (plane->possible_crtcs != drm_crtc_mask(crtc)) {
- DRM_DEBUG_KMS("Cannot change primary plane CRTC\n");
- return -EINVAL;
- }
-
/* Disallow scaling */
src_w >>= 16;
src_h >>= 16;
--
1.8.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] drm/plane-helper: Add drm_plane_helper_check_update() (v3)
2014-05-29 15:06 [PATCH 0/4] Intel primary plane support (v7) Matt Roper
2014-05-29 15:06 ` [PATCH 1/4] drm: Check CRTC compatibility in setplane Matt Roper
@ 2014-05-29 15:06 ` Matt Roper
2014-05-29 15:06 ` [PATCH 3/4] drm/i915: don't force full modeset if primary plane is disabled (v2) Matt Roper
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:06 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
Pull the parameter checking from drm_primary_helper_update() out into
its own function; drivers that provide their own setplane()
implementations rather than using the helper may still want to share
this parameter checking logic.
A few of the checks here were also updated based on suggestions by
Ville Syrjälä.
v3:
- s/primary_helper/plane_helper/ --- this checking logic may be useful
for other types of planes as well.
- Fix visibility check (need to dereference visibility pointer)
v2:
- Pass src/dest/clip rects and min/max scaling down to helper to avoid
duplication of effort between helper and drivers (suggested by
Ville).
- Allow caller to specify whether the primary plane should be
updatable while the crtc is disabled.
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Chon Ming Lee <chon.ming.lee@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/drm_plane_helper.c | 124 ++++++++++++++++++++++++++++---------
include/drm/drm_plane_helper.h | 22 +++++++
2 files changed, 116 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index b601233..fd401fc 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -66,6 +66,79 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
}
/**
+ * drm_plane_helper_check_update() - Check plane update for validity
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @src: source coordinates in 16.16 fixed point
+ * @dest: integer destination coordinates
+ * @clip: integer clipping coordinates
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ * @can_position: is it legal to position the plane such that it
+ * doesn't cover the entire crtc? This will generally
+ * only be false for primary planes.
+ * @can_update_disabled: can the plane be updated while the crtc
+ * is disabled?
+ * @visible: output parameter indicating whether plane is still visible after
+ * clipping
+ *
+ * Checks that a desired plane update is valid. Drivers that provide
+ * their own plane handling rather than helper-provided implementations may
+ * still wish to call this function to avoid duplication of error checking
+ * code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_plane_helper_check_update(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ struct drm_rect *src,
+ struct drm_rect *dest,
+ const struct drm_rect *clip,
+ int min_scale,
+ int max_scale,
+ bool can_position,
+ bool can_update_disabled,
+ bool *visible)
+{
+ int hscale, vscale;
+
+ if (!crtc->enabled && !can_update_disabled) {
+ DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
+ return -EINVAL;
+ }
+
+ /* Check scaling */
+ hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale);
+ vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale);
+ if (hscale < 0 || vscale < 0) {
+ DRM_DEBUG_KMS("Invalid scaling of plane\n");
+ return -ERANGE;
+ }
+
+ *visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
+ if (!*visible)
+ /*
+ * Plane isn't visible; some drivers can handle this
+ * so we just return success here. Drivers that can't
+ * (including those that use the primary plane helper's
+ * update function) will return an error from their
+ * update_plane handler.
+ */
+ return 0;
+
+ if (!can_position && !drm_rect_equals(dest, clip)) {
+ DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_helper_check_update);
+
+/**
* drm_primary_helper_update() - Helper for primary plane update
* @plane: plane object to update
* @crtc: owning CRTC of owning plane
@@ -113,51 +186,42 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
.x = src_x >> 16,
.y = src_y >> 16,
};
+ struct drm_rect src = {
+ .x1 = src_x,
+ .y1 = src_y,
+ .x2 = src_x + src_w,
+ .y2 = src_y + src_h,
+ };
struct drm_rect dest = {
.x1 = crtc_x,
.y1 = crtc_y,
.x2 = crtc_x + crtc_w,
.y2 = crtc_y + crtc_h,
};
- struct drm_rect clip = {
+ const struct drm_rect clip = {
.x2 = crtc->mode.hdisplay,
.y2 = crtc->mode.vdisplay,
};
struct drm_connector **connector_list;
int num_connectors, ret;
+ bool visible;
- if (!crtc->enabled) {
- DRM_DEBUG_KMS("Cannot update primary plane of a disabled CRTC.\n");
- return -EINVAL;
- }
-
- /* Disallow subpixel positioning */
- if ((src_x | src_y | src_w | src_h) & SUBPIXEL_MASK) {
- DRM_DEBUG_KMS("Primary plane does not support subpixel positioning\n");
- return -EINVAL;
- }
-
- /* Disallow scaling */
- src_w >>= 16;
- src_h >>= 16;
- if (crtc_w != src_w || crtc_h != src_h) {
- DRM_DEBUG_KMS("Can't scale primary plane\n");
- return -EINVAL;
- }
-
- /* Make sure primary plane covers entire CRTC */
- drm_rect_intersect(&dest, &clip);
- if (dest.x1 != 0 || dest.y1 != 0 ||
- dest.x2 != crtc->mode.hdisplay || dest.y2 != crtc->mode.vdisplay) {
- DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n");
- return -EINVAL;
- }
-
- /* Framebuffer must be big enough to cover entire plane */
- ret = drm_crtc_check_viewport(crtc, crtc_x, crtc_y, &crtc->mode, fb);
+ ret = drm_plane_helper_check_update(plane, crtc, fb,
+ &src, &dest, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, false, &visible);
if (ret)
return ret;
+ if (!visible)
+ /*
+ * Primary plane isn't visible. Note that unless a driver
+ * provides their own disable function, this will just
+ * wind up returning -EINVAL to userspace.
+ */
+ return plane->funcs->disable_plane(plane);
+
/* Find current connectors for CRTC */
num_connectors = get_connectors_for_crtc(crtc, NULL, 0);
BUG_ON(num_connectors == 0);
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 09824be..a83a646 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -24,6 +24,17 @@
#ifndef DRM_PLANE_HELPER_H
#define DRM_PLANE_HELPER_H
+#include <drm/drm_rect.h>
+
+/*
+ * Drivers that don't allow primary plane scaling may pass this macro in place
+ * of the min/max scale parameters of the update checker function.
+ *
+ * Due to src being in 16.16 fixed point and dest being in integer pixels,
+ * 1<<16 represents no scaling.
+ */
+#define DRM_PLANE_HELPER_NO_SCALING (1<<16)
+
/**
* DOC: plane helpers
*
@@ -31,6 +42,17 @@
* planes.
*/
+extern int drm_plane_helper_check_update(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ struct drm_rect *src,
+ struct drm_rect *dest,
+ const struct drm_rect *clip,
+ int min_scale,
+ int max_scale,
+ bool can_position,
+ bool can_update_disabled,
+ bool *visible);
extern int drm_primary_helper_update(struct drm_plane *plane,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] drm/i915: don't force full modeset if primary plane is disabled (v2)
2014-05-29 15:06 [PATCH 0/4] Intel primary plane support (v7) Matt Roper
2014-05-29 15:06 ` [PATCH 1/4] drm: Check CRTC compatibility in setplane Matt Roper
2014-05-29 15:06 ` [PATCH 2/4] drm/plane-helper: Add drm_plane_helper_check_update() (v3) Matt Roper
@ 2014-05-29 15:06 ` Matt Roper
2014-05-29 15:06 ` [PATCH 4/4] drm/i915: Intel-specific primary plane handling (v8) Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 1/5] kms: Add universal plane support Matt Roper
4 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:06 UTC (permalink / raw)
To: intel-gfx
In a future patch, we'll allow the primary plane to be disabled by
userspace via the universal plane API. If a modeset is requested while
the primary plane is disabled, crtc->primary->fb will be NULL which
generally triggers a full modeset (except in fastboot situations). If
we detect that the crtc is active, but there's no primary plane fb,
we should still allow a simple plane update rather than a full modeset
if the mode isn't actually changing (after re-enabling the primary plane
of course).
v2:
- Enable plane after set_base to avoid enabling the plane if set_base
fails, and to make flip+enable atomic (suggested by Ville)
- Drop BUG to WARN if we somehow enter the 'fb_changed' modeset case
with the crtc disabled (suggested by Ville)
Reviewed-by: Chon Ming Lee <chon.ming.lee@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 731cd01..c2aedac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10544,12 +10544,17 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
if (is_crtc_connector_off(set)) {
config->mode_changed = true;
} else if (set->crtc->primary->fb != set->fb) {
- /* If we have no fb then treat it as a full mode set */
+ /*
+ * If we have no fb, we can only flip as long as the crtc is
+ * active, otherwise we need a full mode set. The crtc may
+ * be active if we've only disabled the primary plane, or
+ * in fastboot situations.
+ */
if (set->crtc->primary->fb == NULL) {
struct intel_crtc *intel_crtc =
to_intel_crtc(set->crtc);
- if (intel_crtc->active && i915.fastboot) {
+ if (intel_crtc->active) {
DRM_DEBUG_KMS("crtc has no fb, will flip\n");
config->fb_changed = true;
} else {
@@ -10787,10 +10792,24 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
ret = intel_set_mode(set->crtc, set->mode,
set->x, set->y, set->fb);
} else if (config->fb_changed) {
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
+
intel_crtc_wait_for_pending_flips(set->crtc);
ret = intel_pipe_set_base(set->crtc,
set->x, set->y, set->fb);
+
+ /*
+ * We need to make sure the primary plane is re-enabled if it
+ * has previously been turned off.
+ */
+ if (!intel_crtc->primary_enabled && ret == 0) {
+ WARN_ON(!intel_crtc->active);
+ intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane,
+ intel_crtc->pipe);
+ }
+
/*
* In the fastboot case this may be our only check of the
* state after boot. It would be better to only do it on
--
1.8.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/4] drm/i915: Intel-specific primary plane handling (v8)
2014-05-29 15:06 [PATCH 0/4] Intel primary plane support (v7) Matt Roper
` (2 preceding siblings ...)
2014-05-29 15:06 ` [PATCH 3/4] drm/i915: don't force full modeset if primary plane is disabled (v2) Matt Roper
@ 2014-05-29 15:06 ` Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 1/5] kms: Add universal plane support Matt Roper
4 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:06 UTC (permalink / raw)
To: intel-gfx
Intel hardware allows the primary plane to be disabled independently of
the CRTC. Provide custom primary plane handling to allow this.
v8:
- Pin/unpin properly when clipping causes the primary plane to be
disabled when it has previously been enabled.
- s/drm_primary_helper_check_update/drm_plane_helper_check_update/
v7:
- Clip primary plane to invisible when crtc is disabled since
intel_crtc->config.pipe_src_{w,h} may be garbage otherwise.
- Unpin old fb before pinning new one in the "just pin and
return" case that is used when the crtc is disabled.
- Don't treat implicit disabling of the primary plane (caused by
clipping) the same way as explicit disabling (caused by fb=0).
For implicit disables, we should leave the fb set and pinned,
whereas for explicit disables we need to unpin the fb before
primary->fb is cleared.
v6:
- Pass rectangles to primary helper check function and get plane
visibility back.
- Wait for pending pageflips on primary plane update/disable.
- Allow primary plane to be updated while the crtc is disabled (changes
will take effect when the crtc is re-enabled if modeset passes -1
for the fb id).
- Drop WARN() if we try to disable the primary plane when it's
already been disabled. This will happen if the crtc gets disabled
after the primary plane has already been disabled independently.
v5:
- Use new drm_primary_helper_check_update() helper function to check
setplane parameter validity.
- Swap primary plane's pipe for pre-gen4 FBC (caught by Ville Syrjälä)
- Cleanup primary plane properly on crtc init failure
v4:
- Don't add a primary_plane field to intel_crtc; that was left over
from a much earlier iteration of this patch series, but is no longer
needed/used now that the DRM core primary plane support has been
merged.
v3:
- Provide gen-specific primary plane format lists (suggested by Daniel
Vetter).
- If the primary plane is already enabled, go ahead and just call the
primary plane helper to do the update (suggested by Daniel Vetter).
- Don't try to disable the primary plane on destruction; the DRM layer
should have already taken care of this for us.
v2:
- Unpin fb properly on primary plane disable
- Provide an Intel-specific set of primary plane formats
- Additional sanity checks on setplane (in line with the checks
currently being done by the DRM core primary plane helper)
Reviewed-by: Chon Ming Lee <chon.ming.lee@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 232 ++++++++++++++++++++++++++++++++++-
1 file changed, 229 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c2aedac..0ddffac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -39,10 +39,37 @@
#include "i915_trace.h"
#include <drm/drm_dp_helper.h>
#include <drm/drm_crtc_helper.h>
+#include <drm/drm_plane_helper.h>
+#include <drm/drm_rect.h>
#include <linux/dma_remapping.h>
+/* Primary plane formats supported by all gen */
+#define COMMON_PRIMARY_FORMATS \
+ DRM_FORMAT_C8, \
+ DRM_FORMAT_RGB565, \
+ DRM_FORMAT_XRGB8888, \
+ DRM_FORMAT_ARGB8888
+
+/* Primary plane formats for gen <= 3 */
+static const uint32_t intel_primary_formats_gen2[] = {
+ COMMON_PRIMARY_FORMATS,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_ARGB1555,
+};
+
+/* Primary plane formats for gen >= 4 */
+static const uint32_t intel_primary_formats_gen4[] = {
+ COMMON_PRIMARY_FORMATS, \
+ DRM_FORMAT_XBGR8888,
+ DRM_FORMAT_ABGR8888,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_ARGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_ABGR2101010,
+};
+
#define DIV_ROUND_CLOSEST_ULL(ll, d) \
- ({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
+({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
static void intel_increase_pllclock(struct drm_crtc *crtc);
static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
@@ -10959,17 +10986,216 @@ static void intel_shared_dpll_init(struct drm_device *dev)
BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
}
+static int
+intel_primary_plane_disable(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct intel_crtc *intel_crtc;
+
+ if (!plane->fb)
+ return 0;
+
+ BUG_ON(!plane->crtc);
+
+ intel_crtc = to_intel_crtc(plane->crtc);
+
+ /*
+ * Even though we checked plane->fb above, it's still possible that
+ * the primary plane has been implicitly disabled because the crtc
+ * coordinates given weren't visible, or because we detected
+ * that it was 100% covered by a sprite plane. Or, the CRTC may be
+ * off and we've set a fb, but haven't actually turned on the CRTC yet.
+ * In either case, we need to unpin the FB and let the fb pointer get
+ * updated, but otherwise we don't need to touch the hardware.
+ */
+ if (!intel_crtc->primary_enabled)
+ goto disable_unpin;
+
+ intel_crtc_wait_for_pending_flips(plane->crtc);
+ intel_disable_primary_hw_plane(dev_priv, intel_plane->plane,
+ intel_plane->pipe);
+
+disable_unpin:
+ intel_unpin_fb_obj(to_intel_framebuffer(plane->fb)->obj);
+ plane->fb = NULL;
+
+ return 0;
+}
+
+static int
+intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
+ struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct drm_rect dest = {
+ /* integer pixels */
+ .x1 = crtc_x,
+ .y1 = crtc_y,
+ .x2 = crtc_x + crtc_w,
+ .y2 = crtc_y + crtc_h,
+ };
+ struct drm_rect src = {
+ /* 16.16 fixed point */
+ .x1 = src_x,
+ .y1 = src_y,
+ .x2 = src_x + src_w,
+ .y2 = src_y + src_h,
+ };
+ const struct drm_rect clip = {
+ /* integer pixels */
+ .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
+ .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
+ };
+ bool visible;
+ int ret;
+
+ ret = drm_plane_helper_check_update(plane, crtc, fb,
+ &src, &dest, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true, &visible);
+
+ if (ret)
+ return ret;
+
+ /*
+ * If the CRTC isn't enabled, we're just pinning the framebuffer,
+ * updating the fb pointer, and returning without touching the
+ * hardware. This allows us to later do a drmModeSetCrtc with fb=-1 to
+ * turn on the display with all planes setup as desired.
+ */
+ if (!crtc->enabled) {
+ /*
+ * If we already called setplane while the crtc was disabled,
+ * we may have an fb pinned; unpin it.
+ */
+ if (plane->fb)
+ intel_unpin_fb_obj(to_intel_framebuffer(plane->fb)->obj);
+
+ /* Pin and return without programming hardware */
+ return intel_pin_and_fence_fb_obj(dev,
+ to_intel_framebuffer(fb)->obj,
+ NULL);
+ }
+
+ intel_crtc_wait_for_pending_flips(crtc);
+
+ /*
+ * If clipping results in a non-visible primary plane, we'll disable
+ * the primary plane. Note that this is a bit different than what
+ * happens if userspace explicitly disables the plane by passing fb=0
+ * because plane->fb still gets set and pinned.
+ */
+ if (!visible) {
+ /*
+ * Try to pin the new fb first so that we can bail out if we
+ * fail.
+ */
+ if (plane->fb != fb) {
+ ret = intel_pin_and_fence_fb_obj(dev,
+ to_intel_framebuffer(fb)->obj,
+ NULL);
+ if (ret)
+ return ret;
+ }
+
+ if (intel_crtc->primary_enabled)
+ intel_disable_primary_hw_plane(dev_priv,
+ intel_plane->plane,
+ intel_plane->pipe);
+
+
+ if (plane->fb != fb)
+ if (plane->fb)
+ intel_unpin_fb_obj(to_intel_framebuffer(plane->fb)->obj);
+
+ return 0;
+ }
+
+ ret = intel_pipe_set_base(crtc, src.x1, src.y1, fb);
+ if (ret)
+ return ret;
+
+ if (!intel_crtc->primary_enabled)
+ intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane,
+ intel_crtc->pipe);
+
+ return 0;
+}
+
+static void intel_primary_plane_destroy(struct drm_plane *plane)
+{
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ drm_plane_cleanup(plane);
+ kfree(intel_plane);
+}
+
+static const struct drm_plane_funcs intel_primary_plane_funcs = {
+ .update_plane = intel_primary_plane_setplane,
+ .disable_plane = intel_primary_plane_disable,
+ .destroy = intel_primary_plane_destroy,
+};
+
+static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
+ int pipe)
+{
+ struct intel_plane *primary;
+ const uint32_t *intel_primary_formats;
+ int num_formats;
+
+ primary = kzalloc(sizeof(*primary), GFP_KERNEL);
+ if (primary == NULL)
+ return NULL;
+
+ primary->can_scale = false;
+ primary->max_downscale = 1;
+ primary->pipe = pipe;
+ primary->plane = pipe;
+ if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
+ primary->plane = !pipe;
+
+ if (INTEL_INFO(dev)->gen <= 3) {
+ intel_primary_formats = intel_primary_formats_gen2;
+ num_formats = ARRAY_SIZE(intel_primary_formats_gen2);
+ } else {
+ intel_primary_formats = intel_primary_formats_gen4;
+ num_formats = ARRAY_SIZE(intel_primary_formats_gen4);
+ }
+
+ drm_universal_plane_init(dev, &primary->base, 0,
+ &intel_primary_plane_funcs,
+ intel_primary_formats, num_formats,
+ DRM_PLANE_TYPE_PRIMARY);
+ return &primary->base;
+}
+
static void intel_crtc_init(struct drm_device *dev, int pipe)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc;
- int i;
+ struct drm_plane *primary;
+ int i, ret;
intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
if (intel_crtc == NULL)
return;
- drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs);
+ primary = intel_primary_plane_create(dev, pipe);
+ ret = drm_crtc_init_with_planes(dev, &intel_crtc->base, primary,
+ NULL, &intel_crtc_funcs);
+ if (ret) {
+ drm_plane_cleanup(primary);
+ kfree(intel_crtc);
+ return;
+ }
drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256);
for (i = 0; i < 256; i++) {
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH i-g-t 1/5] kms: Add universal plane support
2014-05-29 15:06 [PATCH 0/4] Intel primary plane support (v7) Matt Roper
` (3 preceding siblings ...)
2014-05-29 15:06 ` [PATCH 4/4] drm/i915: Intel-specific primary plane handling (v8) Matt Roper
@ 2014-05-29 15:09 ` Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 2/5] kms: Add igt_drm_plane_try_commit() Matt Roper
` (5 more replies)
4 siblings, 6 replies; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:09 UTC (permalink / raw)
To: intel-gfx
Add support for universal planes. This involves revamping the existing
plane handling a bit to allow primary & cursor planes to come from the
DRM plane list, rather than always being manually added. Also,
eliminate the hard-coded position of primary & cursor in the plane list
since the DRM could return them in any order.
To minimize impact for existing tests, we add a new
igt_display_use_universal_commits() call to toggle between universal and
legacy API's for programming the primary and cursor planes.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
lib/igt_kms.c | 132 +++++++++++++++++++++++++++++++++++++++++++++-------------
lib/igt_kms.h | 5 +++
2 files changed, 107 insertions(+), 30 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d00250d..97e329b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -500,27 +500,24 @@ void igt_display_init(igt_display_t *display, int drm_fd)
*/
display->n_pipes = resources->count_crtcs;
+ drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
plane_resources = drmModeGetPlaneResources(display->drm_fd);
igt_assert(plane_resources);
for (i = 0; i < display->n_pipes; i++) {
igt_pipe_t *pipe = &display->pipes[i];
- igt_plane_t *plane;
- int p, j;
+ igt_plane_t *plane = NULL;
+ int p = 0, j;
pipe->display = display;
pipe->pipe = i;
- /* primary plane */
- p = IGT_PLANE_PRIMARY;
- plane = &pipe->planes[p];
- plane->pipe = pipe;
- plane->index = p;
- plane->is_primary = true;
-
/* add the planes that can be used with that pipe */
for (j = 0; j < plane_resources->count_planes; j++) {
drmModePlane *drm_plane;
+ drmModeObjectPropertiesPtr proplist;
+ drmModePropertyPtr prop;
+ int k;
drm_plane = drmModeGetPlane(display->drm_fd,
plane_resources->planes[j]);
@@ -531,21 +528,67 @@ void igt_display_init(igt_display_t *display, int drm_fd)
continue;
}
- p++;
plane = &pipe->planes[p];
plane->pipe = pipe;
- plane->index = p;
+ plane->index = p++;
plane->drm_plane = drm_plane;
+
+ prop = NULL;
+ proplist = drmModeObjectGetProperties(display->drm_fd,
+ plane_resources->planes[j],
+ DRM_MODE_OBJECT_PLANE);
+ for (k = 0; k < proplist->count_props; k++) {
+ prop = drmModeGetProperty(display->drm_fd, proplist->props[k]);
+ if (!prop)
+ continue;
+
+ if (strcmp(prop->name, "type") != 0) {
+ drmModeFreeProperty(prop);
+ continue;
+ }
+
+ switch (proplist->prop_values[k]) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ plane->is_primary = 1;
+ display->has_universal_planes = 1;
+ break;
+ case DRM_PLANE_TYPE_CURSOR:
+ plane->is_cursor = 1;
+ pipe->has_cursor = 1;
+ display->has_universal_planes = 1;
+ break;
+ }
+
+ drmModeFreeProperty(prop);
+ break;
+ }
+
}
- /* cursor plane */
- p++;
- plane = &pipe->planes[p];
- plane->pipe = pipe;
- plane->index = p;
- plane->is_cursor = true;
+ /*
+ * Add a drm_plane-less primary plane for kernels without
+ * universal plane support.
+ */
+ if (!display->has_universal_planes) {
+ plane = &pipe->planes[p];
+ plane->pipe = pipe;
+ plane->index = p++;
+ plane->is_primary = true;
+ }
- pipe->n_planes = ++p;
+ /*
+ * Add drm_plane-less cursor plane for kernels that don't
+ * expose a universal cursor plane.
+ */
+ if (!pipe->has_cursor) {
+ /* cursor plane */
+ plane = &pipe->planes[p];
+ plane->pipe = pipe;
+ plane->index = p++;
+ plane->is_cursor = true;
+ }
+
+ pipe->n_planes = p;
/* make sure we don't overflow the plane array */
igt_assert(pipe->n_planes <= IGT_MAX_PLANES);
@@ -700,16 +743,29 @@ static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, enum igt_plane plane)
{
int idx;
- /* Cursor plane is always the upper plane */
- if (plane == IGT_PLANE_CURSOR)
- idx = pipe->n_planes - 1;
- else {
- igt_assert_f(plane >= 0 && plane < (pipe->n_planes),
- "plane=%d\n", plane);
- idx = plane;
+ for (idx = 0; idx < pipe->n_planes; idx++) {
+ switch (plane) {
+ case IGT_PLANE_PRIMARY:
+ if (pipe->planes[idx].is_primary)
+ return &pipe->planes[idx];
+ break;
+ case IGT_PLANE_CURSOR:
+ if (pipe->planes[idx].is_cursor)
+ return &pipe->planes[idx];
+ break;
+ case IGT_PLANE_2:
+ if (!pipe->planes[idx].is_primary &&
+ !pipe->planes[idx].is_cursor)
+ return &pipe->planes[idx];
+ default:
+ if (!pipe->planes[idx].is_primary &&
+ !pipe->planes[idx].is_cursor)
+ /* Consume this overlay and keep searching for another */
+ plane--;
+ }
}
- return &pipe->planes[idx];
+ return NULL;
}
static uint32_t igt_plane_get_fb_id(igt_plane_t *plane)
@@ -761,6 +817,8 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
uint32_t fb_id, crtc_id;
int ret;
+ igt_assert(plane->drm_plane);
+
fb_id = igt_plane_get_fb_id(plane);
crtc_id = output->config.crtc->crtc_id;
pipe = igt_output_get_driving_pipe(output);
@@ -820,7 +878,11 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
static int igt_plane_commit(igt_plane_t *plane, igt_output_t *output)
{
- if (plane->is_cursor) {
+ struct igt_display *display = plane->pipe->display;
+
+ if (display->commit_universal && plane->drm_plane) {
+ igt_drm_plane_commit(plane, output);
+ } else if (plane->is_cursor) {
igt_cursor_commit(plane, output);
} else if (plane->is_primary) {
/* state updated by SetCrtc */
@@ -838,8 +900,8 @@ static int igt_output_commit(igt_output_t *output)
int i;
pipe = igt_output_get_driving_pipe(output);
- if (pipe->need_set_crtc) {
- igt_plane_t *primary = &pipe->planes[0];
+ if (pipe->need_set_crtc && !display->commit_universal) {
+ igt_plane_t *primary = igt_pipe_get_plane(pipe, IGT_PLANE_PRIMARY);
drmModeModeInfo *mode;
uint32_t fb_id, crtc_id;
int ret;
@@ -889,7 +951,7 @@ static int igt_output_commit(igt_output_t *output)
primary->fb_changed = false;
}
- if (pipe->need_set_cursor) {
+ if (pipe->need_set_cursor && !display->commit_universal) {
igt_plane_t *cursor;
uint32_t gem_handle, crtc_id;
int ret;
@@ -940,6 +1002,16 @@ static int igt_output_commit(igt_output_t *output)
return 0;
}
+/*
+ * Indicate whether future display commits should use universal plane API's
+ * or legacy API's.
+ */
+void igt_display_use_universal_commits(igt_display_t *display,
+ bool use_universal)
+{
+ display->commit_universal = use_universal;
+}
+
int igt_display_commit(igt_display_t *display)
{
int i;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 8e80d4b..4955fc8 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -121,6 +121,7 @@ struct igt_pipe {
unsigned int need_set_crtc : 1;
unsigned int need_set_cursor : 1;
unsigned int need_wait_for_vblank : 1;
+ unsigned int has_cursor : 1;
#define IGT_MAX_PLANES 4
int n_planes;
igt_plane_t planes[IGT_MAX_PLANES];
@@ -143,6 +144,8 @@ struct igt_display {
unsigned long pipes_in_use;
igt_output_t *outputs;
igt_pipe_t pipes[I915_MAX_PIPES];
+ bool has_universal_planes;
+ bool commit_universal;
};
/* set vt into graphics mode, required to prevent fbcon from interfering */
@@ -150,6 +153,8 @@ void igt_set_vt_graphics_mode(void);
void igt_display_init(igt_display_t *display, int drm_fd);
void igt_display_fini(igt_display_t *display);
+void igt_display_use_universal_commits(igt_display_t *display,
+ bool use_universal);
int igt_display_commit(igt_display_t *display);
int igt_display_get_n_pipes(igt_display_t *display);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH i-g-t 2/5] kms: Add igt_drm_plane_try_commit()
2014-05-29 15:09 ` [PATCH i-g-t 1/5] kms: Add universal plane support Matt Roper
@ 2014-05-29 15:09 ` Matt Roper
2014-06-13 16:24 ` Damien Lespiau
2014-05-29 15:09 ` [PATCH i-g-t 3/5] kms_plane: Update for universal plane changes Matt Roper
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:09 UTC (permalink / raw)
To: intel-gfx
For some of our tests, we want to make sure that bogus plane programming
attempts fail with the expected error code. Add
igt_drm_plane_try_commit() that will just return the error code on
failure rather than failing the current subtest. This lets us test the
return value against the expected error code.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
lib/igt_kms.c | 21 ++++++++++++++++++---
lib/igt_kms.h | 1 +
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 97e329b..ce6ea87 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -810,7 +810,11 @@ static int igt_cursor_commit(igt_plane_t *plane, igt_output_t *output)
return 0;
}
-static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
+/*
+ * Attempt to commit a plane; if the DRM call to program the plane fails,
+ * just return an error code (but don't fail the current subtest).
+ */
+int igt_drm_plane_try_commit(igt_plane_t *plane, igt_output_t *output)
{
igt_display_t *display = output->display;
igt_pipe_t *pipe;
@@ -842,7 +846,8 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
IGT_FIXED(0,0), /* src_w */
IGT_FIXED(0,0) /* src_h */);
- igt_assert(ret == 0);
+ if (ret)
+ return ret;
plane->fb_changed = false;
} else if (plane->fb_changed || plane->position_changed) {
@@ -866,7 +871,8 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
IGT_FIXED(plane->fb->width,0), /* src_w */
IGT_FIXED(plane->fb->height,0) /* src_h */);
- igt_assert(ret == 0);
+ if (ret)
+ return ret;
plane->fb_changed = false;
plane->position_changed = false;
@@ -876,6 +882,15 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
return 0;
}
+static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
+{
+ int ret = igt_drm_plane_try_commit(plane, output);
+
+ igt_assert(ret == 0);
+
+ return 0;
+}
+
static int igt_plane_commit(igt_plane_t *plane, igt_output_t *output)
{
struct igt_display *display = plane->pipe->display;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 4955fc8..2f72a1c 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -157,6 +157,7 @@ void igt_display_use_universal_commits(igt_display_t *display,
bool use_universal);
int igt_display_commit(igt_display_t *display);
int igt_display_get_n_pipes(igt_display_t *display);
+int igt_drm_plane_try_commit(igt_plane_t *plane, igt_output_t *output);
const char *igt_output_name(igt_output_t *output);
drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH i-g-t 3/5] kms_plane: Update for universal plane changes
2014-05-29 15:09 ` [PATCH i-g-t 1/5] kms: Add universal plane support Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 2/5] kms: Add igt_drm_plane_try_commit() Matt Roper
@ 2014-05-29 15:09 ` Matt Roper
2014-06-13 16:27 ` Damien Lespiau
2014-05-29 15:09 ` [PATCH i-g-t 4/5] kms_universal_plane: Universal plane testing (v5) Matt Roper
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:09 UTC (permalink / raw)
To: intel-gfx
Recent changes in igt_kms to support universal planes have removed the
plane list order guarantees that kms_plane depended upon. Ensure that
we loop over the entire plane list properly and then selectively skip
non-overlay planes.
While we're at it, update a couple igt_output_get_plane() calls to use
plane enums rather than integer values to make it clear what we're
actually doing.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
tests/kms_plane.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 5db0947..1d334ab 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -94,7 +94,7 @@ test_position_init(test_position_t *test, igt_output_t *output, enum pipe pipe)
test->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
igt_output_set_pipe(output, pipe);
- primary = igt_output_get_plane(output, 0);
+ primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
mode = igt_output_get_mode(output);
igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
@@ -146,9 +146,14 @@ test_plane_position_with_output(data_t *data,
test_position_init(&test, output, pipe);
mode = igt_output_get_mode(output);
- primary = igt_output_get_plane(output, 0);
+ primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
sprite = igt_output_get_plane(output, plane);
+ if (sprite->is_primary) {
+ test_position_fini(&test, output);
+ igt_skip("Skipping primary plane\n");
+ }
+
create_fb_for_mode__position(data, mode, 100, 100, 64, 64,
&primary_fb);
igt_plane_set_fb(primary, &primary_fb);
@@ -213,7 +218,7 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
{
int plane;
- for (plane = 1; plane < IGT_MAX_PLANES; plane++)
+ for (plane = 0; plane < IGT_MAX_PLANES; plane++)
run_tests_for_pipe_plane(data, pipe, plane);
}
--
1.8.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH i-g-t 4/5] kms_universal_plane: Universal plane testing (v5)
2014-05-29 15:09 ` [PATCH i-g-t 1/5] kms: Add universal plane support Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 2/5] kms: Add igt_drm_plane_try_commit() Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 3/5] kms_plane: Update for universal plane changes Matt Roper
@ 2014-05-29 15:09 ` Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 5/5] kms_cursor_crc: Combine data_t and test_data_t Matt Roper
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:09 UTC (permalink / raw)
To: intel-gfx
Add a simple test to exercise universal plane support.
v5:
- Check that we don't have more than one primary or cursor. This will
catch accidental calls to drm_plane_init() in the kernel where
drm_universal_plane_init() was intended (these don't cause a compile
warning due to type compatibility between enum and bool).
v4:
- Test disabling the primary plane explicitly when it has previously
been implicitly disabled due to clipping.
- Skip test if igt_pipe_crc_new() fails
v3:
- For testing while crtc is off, switch between several different
primary plane fb's before reenabling the crtc. This will help
catch pin/unpin mistakes.
v2:
- Test that pageflips error out gracefully when the primary plane
is disabled before the ioctl, or between ioctl and pageflip
execution.
- Test that nothing blows up if we try to disable the primary plane
immediately after a pageflip (presumably before the pageflip actually
completes).
- Test that we can setup primary + sprite planes with the CRTC off and
then have them show up properly when we enable the CRTC
(drmModeSetCrtc with fb = -1).
- Test that we can modeset properly after having disabled the primary
plane
- Test that proper error codes are returned for invalid plane
programming attempts.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
tests/.gitignore | 1 +
tests/Makefile.sources | 1 +
tests/kms_universal_plane.c | 598 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 600 insertions(+)
create mode 100644 tests/kms_universal_plane.c
diff --git a/tests/.gitignore b/tests/.gitignore
index d7ad054..d1b01f7 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -124,6 +124,7 @@ kms_pipe_crc_basic
kms_plane
kms_render
kms_setmode
+kms_universal_plane
multi-tests.txt
pm_lpsp
pm_rpm
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index eca4af9..0132f8e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -70,6 +70,7 @@ TESTS_progs_M = \
kms_plane \
kms_render \
kms_setmode \
+ kms_universal_plane \
pm_lpsp \
pm_rpm \
pm_rps \
diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
new file mode 100644
index 0000000..df48b50
--- /dev/null
+++ b/tests/kms_universal_plane.c
@@ -0,0 +1,598 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+
+typedef struct {
+ int drm_fd;
+ igt_display_t display;
+} data_t;
+
+typedef struct {
+ data_t *data;
+ igt_pipe_crc_t *pipe_crc;
+ igt_crc_t crc_1, crc_2, crc_3, crc_4, crc_5, crc_6, crc_7, crc_8,
+ crc_9, crc_10;
+ struct igt_fb red_fb, blue_fb, black_fb, yellow_fb;
+ drmModeModeInfo *mode;
+} functional_test_t;
+
+typedef struct {
+ data_t *data;
+ drmModeResPtr moderes;
+ struct igt_fb blue_fb, oversized_fb, undersized_fb;
+} sanity_test_t;
+
+typedef struct {
+ data_t *data;
+ struct igt_fb red_fb, blue_fb;
+} pageflip_test_t;
+
+static void
+functional_test_init(functional_test_t *test, igt_output_t *output, enum pipe pipe)
+{
+ data_t *data = test->data;
+ drmModeModeInfo *mode;
+
+ test->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
+ if (!test->pipe_crc)
+ igt_skip("auto crc not supported on this connector with pipe %i\n",
+ pipe);
+
+
+ igt_output_set_pipe(output, pipe);
+
+ mode = igt_output_get_mode(output);
+ igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 0.0, 0.0, 0.0,
+ &test->black_fb);
+ igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 0.0, 0.0, 1.0,
+ &test->blue_fb);
+ igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 1.0, 1.0, 0.0,
+ &test->yellow_fb);
+ igt_create_color_fb(data->drm_fd, 100, 100,
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 1.0, 0.0, 0.0,
+ &test->red_fb);
+
+ test->mode = mode;
+}
+
+static void
+functional_test_fini(functional_test_t *test, igt_output_t *output)
+{
+ igt_pipe_crc_free(test->pipe_crc);
+
+ igt_remove_fb(test->data->drm_fd, &test->black_fb);
+ igt_remove_fb(test->data->drm_fd, &test->blue_fb);
+ igt_remove_fb(test->data->drm_fd, &test->red_fb);
+ igt_remove_fb(test->data->drm_fd, &test->yellow_fb);
+
+ igt_display_use_universal_commits(&test->data->display, false);
+ igt_output_set_pipe(output, PIPE_ANY);
+ igt_display_commit(&test->data->display);
+}
+
+/*
+ * Universal plane functional testing.
+ * - Black primary plane via traditional interfaces, red sprite, grab CRC:1.
+ * - Blue primary plane via traditional interfaces, red sprite, grab CRC:2.
+ * - Yellow primary via traditional interfaces
+ * - Blue primary plane, red sprite via universal planes, grab CRC:3 and compare
+ * with CRC:2 (should be the same)
+ * - Disable primary plane, grab CRC:4 (should be same as CRC:1)
+ * - Reenable primary, grab CRC:5 (should be same as CRC:2 and CRC:3)
+ * - Yellow primary, no sprite
+ * - Disable CRTC
+ * - Program red sprite (while CRTC off)
+ * - Program blue primary (while CRTC off)
+ * - Enable CRTC, grab CRC:6 (should be same as CRC:2)
+ */
+static void
+functional_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
+{
+ functional_test_t test = { .data = data };
+ igt_display_t *display = &data->display;
+ igt_plane_t *primary, *sprite;
+ int ret;
+ int num_primary = 0, num_cursor = 0;
+ int i;
+
+ igt_skip_on(pipe >= display->n_pipes);
+
+ fprintf(stdout, "Testing connector %s using pipe %c\n",
+ igt_output_name(output), pipe_name(pipe));
+
+ functional_test_init(&test, output, pipe);
+
+ /*
+ * Make sure we have no more than one primary or cursor plane per crtc.
+ * If the kernel accidentally calls drm_plane_init() rather than
+ * drm_universal_plane_init(), the type enum can get interpreted as a
+ * boolean and show up in userspace as the wrong type.
+ */
+ for (i = 0; i < display->pipes[pipe].n_planes; i++)
+ if (display->pipes[pipe].planes[i].is_primary)
+ num_primary++;
+ else if (display->pipes[pipe].planes[i].is_cursor)
+ num_cursor++;
+
+ igt_assert(num_primary == 1);
+ igt_assert(num_cursor <= 1);
+
+ primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+ sprite = igt_output_get_plane(output, IGT_PLANE_2);
+ if (!sprite) {
+ functional_test_fini(&test, output);
+ igt_skip("No sprite plane available\n");
+ }
+
+ igt_plane_set_position(sprite, 100, 100);
+
+ /* Step 1: Legacy API's, black primary, red sprite (CRC 1) */
+ igt_plane_set_fb(primary, &test.black_fb);
+ igt_plane_set_fb(sprite, &test.red_fb);
+ igt_display_commit(display);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_1);
+
+ /* Step 2: Legacy API', blue primary, red sprite (CRC 2) */
+ igt_plane_set_fb(primary, &test.blue_fb);
+ igt_plane_set_fb(sprite, &test.red_fb);
+ igt_display_commit(display);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_2);
+
+ /* Step 3: Legacy API's, yellow primary (CRC 3) */
+ igt_plane_set_fb(primary, &test.yellow_fb);
+ igt_display_commit(display);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_3);
+
+ /* Step 4: Universal API's, blue primary, red sprite (CRC 4) */
+ igt_display_use_universal_commits(display, true);
+ igt_plane_set_fb(primary, &test.blue_fb);
+ igt_plane_set_fb(sprite, &test.red_fb);
+ igt_display_commit(display);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_4);
+
+ /* Step 5: Universal API's, disable primary plane (CRC 5) */
+ igt_plane_set_fb(primary, NULL);
+ igt_display_commit(display);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_5);
+
+ /* Step 6: Universal API's, re-enable primary with blue (CRC 6) */
+ igt_display_use_universal_commits(display, true);
+ igt_plane_set_fb(primary, &test.blue_fb);
+ igt_display_commit(display);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_6);
+
+ /* Step 7: Legacy API's, yellow primary, no sprite */
+ igt_display_use_universal_commits(display, false);
+ igt_plane_set_fb(primary, &test.yellow_fb);
+ igt_plane_set_fb(sprite, NULL);
+ igt_display_commit(display);
+
+ /* Step 8: Disable CRTC */
+ igt_plane_set_fb(primary, NULL);
+ igt_display_commit(display);
+
+ /* Step 9: Universal API's with crtc off:
+ * - red sprite
+ * - multiple primary fb's, ending in blue
+ */
+ igt_display_use_universal_commits(display, true);
+ igt_plane_set_fb(sprite, &test.red_fb);
+ ret = igt_drm_plane_try_commit(sprite, output);
+ igt_assert(ret == 0);
+ igt_plane_set_fb(primary, &test.yellow_fb);
+ ret = igt_drm_plane_try_commit(primary, output);
+ igt_plane_set_fb(primary, &test.black_fb);
+ ret = igt_drm_plane_try_commit(primary, output);
+ igt_plane_set_fb(primary, &test.blue_fb);
+ ret = igt_drm_plane_try_commit(primary, output);
+ igt_assert(ret == 0);
+ display->pipes[output->config.pipe].need_wait_for_vblank = false;
+
+ /* Step 10: Enable crtc (fb = -1), take CRC (CRC 7) */
+ ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id, -1,
+ 0, 0, &output->config.connector->connector_id,
+ 1, test.mode);
+ igt_assert(ret == 0);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_7);
+
+ /* Step 11: Disable primary plane */
+ igt_plane_set_fb(primary, NULL);
+ igt_display_commit(display);
+
+ /* Step 12: Legacy modeset to yellow FB (CRC 8) */
+ igt_display_use_universal_commits(display, false);
+ igt_plane_set_fb(primary, &test.yellow_fb);
+ igt_display_commit(display);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_8);
+
+ /* Step 13: Legacy API', blue primary, red sprite */
+ igt_plane_set_fb(primary, &test.blue_fb);
+ igt_plane_set_fb(sprite, &test.red_fb);
+ igt_display_commit(display);
+
+ /* Step 14: Universal API, set primary completely offscreen (CRC 9) */
+ ret = drmModeSetPlane(data->drm_fd, primary->drm_plane->plane_id,
+ output->config.crtc->crtc_id,
+ test.blue_fb.fb_id, 0,
+ 9000, 9000,
+ test.mode->hdisplay,
+ test.mode->vdisplay,
+ IGT_FIXED(0,0), IGT_FIXED(0,0),
+ IGT_FIXED(test.mode->hdisplay,0),
+ IGT_FIXED(test.mode->vdisplay,0));
+ igt_assert(ret == 0);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_9);
+
+ /*
+ * Step 15: Explicitly disable primary after it's already been
+ * implicitly disabled (CRC 10).
+ */
+ igt_display_use_universal_commits(display, true);
+ igt_plane_set_fb(primary, NULL);
+ igt_display_commit(display);
+ igt_pipe_crc_collect_crc(test.pipe_crc, &test.crc_10);
+
+ /* Step 16: Legacy API's, blue primary, red sprite */
+ igt_display_use_universal_commits(display, false);
+ igt_plane_set_fb(primary, &test.blue_fb);
+ igt_plane_set_fb(sprite, &test.red_fb);
+ igt_display_commit(display);
+
+ /* Blue bg + red sprite should be same under both types of API's */
+ igt_assert(igt_crc_equal(&test.crc_2, &test.crc_4));
+
+ /* Disabling primary plane should be same as black primary */
+ igt_assert(igt_crc_equal(&test.crc_1, &test.crc_5));
+
+ /* Re-enabling primary should return to blue properly */
+ igt_assert(igt_crc_equal(&test.crc_2, &test.crc_6));
+
+ /*
+ * We should be able to setup plane FB's while CRTC is disabled and
+ * then have them pop up correctly when the CRTC is re-enabled.
+ */
+ igt_assert(igt_crc_equal(&test.crc_2, &test.crc_7));
+
+ /*
+ * We should be able to modeset with the primary plane off
+ * successfully
+ */
+ igt_assert(igt_crc_equal(&test.crc_3, &test.crc_8));
+
+ /*
+ * We should be able to move the primary plane completely offscreen
+ * and have it disable successfully.
+ */
+ igt_assert(igt_crc_equal(&test.crc_5, &test.crc_9));
+
+ /*
+ * We should be able to explicitly disable an already
+ * implicitly-disabled primary plane
+ */
+ igt_assert(igt_crc_equal(&test.crc_5, &test.crc_10));
+
+ igt_plane_set_fb(primary, NULL);
+ igt_plane_set_fb(sprite, NULL);
+
+ functional_test_fini(&test, output);
+}
+
+static void
+sanity_test_init(sanity_test_t *test, igt_output_t *output, enum pipe pipe)
+{
+ data_t *data = test->data;
+ drmModeModeInfo *mode;
+
+ igt_output_set_pipe(output, pipe);
+
+ mode = igt_output_get_mode(output);
+ igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 0.0, 0.0, 1.0,
+ &test->blue_fb);
+ igt_create_color_fb(data->drm_fd,
+ mode->hdisplay + 100, mode->vdisplay + 100,
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 0.0, 0.0, 1.0,
+ &test->oversized_fb);
+ igt_create_color_fb(data->drm_fd,
+ mode->hdisplay - 100, mode->vdisplay - 100,
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 0.0, 0.0, 1.0,
+ &test->undersized_fb);
+
+ test->moderes = drmModeGetResources(data->drm_fd);
+}
+
+static void
+sanity_test_fini(sanity_test_t *test, igt_output_t *output)
+{
+ drmModeFreeResources(test->moderes);
+
+ igt_remove_fb(test->data->drm_fd, &test->oversized_fb);
+ igt_remove_fb(test->data->drm_fd, &test->undersized_fb);
+ igt_remove_fb(test->data->drm_fd, &test->blue_fb);
+
+ igt_display_use_universal_commits(&test->data->display, false);
+ igt_output_set_pipe(output, PIPE_ANY);
+ igt_display_commit(&test->data->display);
+}
+
+/*
+ * Universal plane sanity testing.
+ * - Primary doesn't cover CRTC
+ * - Primary plane tries to scale down
+ * - Primary plane tries to scale up
+ */
+static void
+sanity_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
+{
+ sanity_test_t test = { .data = data };
+ igt_plane_t *primary;
+ drmModeModeInfo *mode;
+ int i, ret = 0;
+
+ igt_output_set_pipe(output, pipe);
+ mode = igt_output_get_mode(output);
+
+ sanity_test_init(&test, output, pipe);
+
+ primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+
+ /* Use legacy API to set a mode with a blue FB */
+ igt_display_use_universal_commits(&data->display, false);
+ igt_plane_set_fb(primary, &test.blue_fb);
+ igt_display_commit(&data->display);
+
+ /*
+ * Try to use universal plane API to set primary plane that
+ * doesn't cover CRTC (should fail).
+ */
+ igt_plane_set_fb(primary, &test.undersized_fb);
+ ret = igt_drm_plane_try_commit(primary, output);
+ igt_assert(ret == -EINVAL);
+
+ /* Same as above, but different plane positioning. */
+ primary->crtc_x = 100;
+ primary->crtc_y = 100;
+ primary->position_changed = true;
+ ret = igt_drm_plane_try_commit(primary, output);
+ igt_assert(ret == -EINVAL);
+
+ primary->crtc_x = 0;
+ primary->crtc_y = 0;
+ primary->position_changed = false;
+
+ /* Try to use universal plane API to scale down (should fail) */
+ ret = drmModeSetPlane(data->drm_fd, primary->drm_plane->plane_id,
+ output->config.crtc->crtc_id,
+ test.oversized_fb.fb_id, 0,
+ 0, 0,
+ mode->hdisplay + 100,
+ mode->vdisplay + 100,
+ IGT_FIXED(0,0), IGT_FIXED(0,0),
+ IGT_FIXED(mode->hdisplay,0),
+ IGT_FIXED(mode->vdisplay,0));
+ igt_assert(ret == -ERANGE);
+
+ /* Try to use universal plane API to scale up (should fail) */
+ ret = drmModeSetPlane(data->drm_fd, primary->drm_plane->plane_id,
+ output->config.crtc->crtc_id,
+ test.oversized_fb.fb_id, 0,
+ 0, 0,
+ mode->hdisplay,
+ mode->vdisplay,
+ IGT_FIXED(0,0), IGT_FIXED(0,0),
+ IGT_FIXED(mode->hdisplay - 100,0),
+ IGT_FIXED(mode->vdisplay - 100,0));
+ igt_assert(ret == -ERANGE);
+
+ /* Find other crtcs and try to program our primary plane on them */
+ for (i = 0; i < test.moderes->count_crtcs; i++)
+ if (test.moderes->crtcs[i] != output->config.crtc->crtc_id) {
+ ret = drmModeSetPlane(data->drm_fd,
+ primary->drm_plane->plane_id,
+ test.moderes->crtcs[i],
+ test.blue_fb.fb_id, 0,
+ 0, 0,
+ mode->hdisplay,
+ mode->vdisplay,
+ IGT_FIXED(0,0), IGT_FIXED(0,0),
+ IGT_FIXED(mode->hdisplay,0),
+ IGT_FIXED(mode->vdisplay,0));
+ igt_assert(ret == -EINVAL);
+ }
+
+ igt_plane_set_fb(primary, NULL);
+ sanity_test_fini(&test, output);
+}
+
+static void
+pageflip_test_init(pageflip_test_t *test, igt_output_t *output, enum pipe pipe)
+{
+ data_t *data = test->data;
+ drmModeModeInfo *mode;
+
+ igt_output_set_pipe(output, pipe);
+
+ mode = igt_output_get_mode(output);
+ igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 1.0, 0.0, 0.0,
+ &test->red_fb);
+ igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 0.0, 0.0, 1.0,
+ &test->blue_fb);
+}
+
+static void
+pageflip_test_fini(pageflip_test_t *test, igt_output_t *output)
+{
+ igt_remove_fb(test->data->drm_fd, &test->red_fb);
+ igt_remove_fb(test->data->drm_fd, &test->blue_fb);
+
+ igt_display_use_universal_commits(&test->data->display, false);
+ igt_output_set_pipe(output, PIPE_ANY);
+ igt_display_commit(&test->data->display);
+}
+
+static void
+pageflip_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
+{
+ pageflip_test_t test = { .data = data };
+ igt_plane_t *primary;
+ struct timeval timeout = { .tv_sec = 0, .tv_usec = 500 };
+ drmEventContext evctx = { .version = DRM_EVENT_CONTEXT_VERSION };
+
+ fd_set fds;
+ int ret = 0;
+
+ igt_output_set_pipe(output, pipe);
+
+ pageflip_test_init(&test, output, pipe);
+
+ primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+
+ /* Use legacy API to set a mode with a blue FB */
+ igt_display_use_universal_commits(&data->display, false);
+ igt_plane_set_fb(primary, &test.blue_fb);
+ igt_display_commit(&data->display);
+
+ /* Disable the primary plane */
+ igt_display_use_universal_commits(&data->display, true);
+ igt_plane_set_fb(primary, NULL);
+ igt_display_commit(&data->display);
+
+ /*
+ * Issue a pageflip to red FB
+ *
+ * Note that crtc->primary->fb = NULL causes flip to return EBUSY for
+ * historical reasons...
+ */
+ ret = drmModePageFlip(data->drm_fd, output->config.crtc->crtc_id,
+ test.red_fb.fb_id, 0, NULL);
+ igt_assert(ret == -EBUSY);
+
+ /* Turn primary plane back on */
+ igt_plane_set_fb(primary, &test.blue_fb);
+ igt_display_commit(&data->display);
+
+ /*
+ * Issue a pageflip to red, then immediately try to disable the primary
+ * plane, hopefully before the pageflip has a chance to complete. The
+ * plane disable operation should wind up blocking while the pageflip
+ * completes, which we don't have a good way to specifically test for,
+ * but at least we can make sure that nothing blows up.
+ */
+ ret = drmModePageFlip(data->drm_fd, output->config.crtc->crtc_id,
+ test.red_fb.fb_id, DRM_MODE_PAGE_FLIP_EVENT,
+ &test);
+ igt_assert(ret == 0);
+ igt_plane_set_fb(primary, NULL);
+ igt_display_commit(&data->display);
+
+ /* Wait for pageflip completion, then consume event on fd */
+ FD_ZERO(&fds);
+ FD_SET(data->drm_fd, &fds);
+ do {
+ ret = select(data->drm_fd + 1, &fds, NULL, NULL, &timeout);
+ } while (ret < 0 && errno == EINTR);
+ igt_assert(ret == 1);
+ ret = drmHandleEvent(data->drm_fd, &evctx);
+ igt_assert(ret == 0);
+
+ igt_display_use_universal_commits(&data->display, false);
+ igt_plane_set_fb(primary, NULL);
+ pageflip_test_fini(&test, output);
+}
+
+static void
+run_tests_for_pipe(data_t *data, enum pipe pipe)
+{
+ igt_output_t *output;
+
+ igt_assert(data->display.has_universal_planes);
+
+ igt_subtest_f("universal-plane-pipe-%c-functional", pipe_name(pipe))
+ for_each_connected_output(&data->display, output)
+ functional_test_pipe(data, pipe, output);
+
+ igt_subtest_f("universal-plane-pipe-%c-sanity", pipe_name(pipe))
+ for_each_connected_output(&data->display, output)
+ sanity_test_pipe(data, pipe, output);
+
+ igt_subtest_f("disable-primary-vs-flip-pipe-%c", pipe_name(pipe))
+ for_each_connected_output(&data->display, output)
+ pageflip_test_pipe(data, pipe, output);
+}
+
+static data_t data;
+
+igt_main
+{
+
+ igt_skip_on_simulation();
+
+ igt_fixture {
+ data.drm_fd = drm_open_any();
+
+ igt_set_vt_graphics_mode();
+
+ igt_require_pipe_crc();
+ igt_display_init(&data.display, data.drm_fd);
+
+ igt_require(data.display.has_universal_planes);
+ }
+
+ for (int pipe = 0; pipe < 3; pipe++)
+ run_tests_for_pipe(&data, pipe);
+
+ igt_fixture {
+ igt_display_fini(&data.display);
+ }
+}
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH i-g-t 5/5] kms_cursor_crc: Combine data_t and test_data_t
2014-05-29 15:09 ` [PATCH i-g-t 1/5] kms: Add universal plane support Matt Roper
` (2 preceding siblings ...)
2014-05-29 15:09 ` [PATCH i-g-t 4/5] kms_universal_plane: Universal plane testing (v5) Matt Roper
@ 2014-05-29 15:09 ` Matt Roper
2014-06-13 16:21 ` [PATCH i-g-t 1/5] kms: Add universal plane support Damien Lespiau
2014-06-13 17:00 ` Damien Lespiau
5 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2014-05-29 15:09 UTC (permalink / raw)
To: intel-gfx
If a subtest fails, cleanup_crtc() never gets called and then the
test_data_t structure for the test is lost, including the CRC file
descriptor that we never got a chance to release; this causes all
subsequent tests to fail with -EBUSY at igt_pipe_crc_new().
The split between permanent data_t and temporary test_data_t doesn't
seem to serve a purpose, so just combine the fields from both into
data_t. This will prevent us from losing the CRC filedescriptor so that
we can properly close and reopen it after a failed test.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
tests/kms_cursor_crc.c | 206 +++++++++++++++++++++++--------------------------
1 file changed, 97 insertions(+), 109 deletions(-)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 06625ee..92d9a3c 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -44,10 +44,6 @@ typedef struct {
igt_display_t display;
struct igt_fb primary_fb;
struct igt_fb fb;
-} data_t;
-
-typedef struct {
- data_t *data;
igt_output_t *output;
enum pipe pipe;
igt_crc_t ref_crc;
@@ -55,7 +51,7 @@ typedef struct {
int screenw, screenh;
int curw, curh; /* cursor size */
igt_pipe_crc_t *pipe_crc;
-} test_data_t;
+} data_t;
static void draw_cursor(cairo_t *cr, int x, int y, int w)
{
@@ -71,11 +67,10 @@ static void draw_cursor(cairo_t *cr, int x, int y, int w)
igt_paint_color_alpha(cr, x + w, y + w, w, w, 0.5, 0.5, 0.5, 1.0);
}
-static void cursor_enable(test_data_t *test_data)
+static void cursor_enable(data_t *data)
{
- data_t *data = test_data->data;
igt_display_t *display = &data->display;
- igt_output_t *output = test_data->output;
+ igt_output_t *output = data->output;
igt_plane_t *cursor;
cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
@@ -83,11 +78,10 @@ static void cursor_enable(test_data_t *test_data)
igt_display_commit(display);
}
-static void cursor_disable(test_data_t *test_data)
+static void cursor_disable(data_t *data)
{
- data_t *data = test_data->data;
igt_display_t *display = &data->display;
- igt_output_t *output = test_data->output;
+ igt_output_t *output = data->output;
igt_plane_t *cursor;
cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
@@ -96,11 +90,10 @@ static void cursor_disable(test_data_t *test_data)
}
-static void do_single_test(test_data_t *test_data, int x, int y)
+static void do_single_test(data_t *data, int x, int y)
{
- data_t *data = test_data->data;
igt_display_t *display = &data->display;
- igt_pipe_crc_t *pipe_crc = test_data->pipe_crc;
+ igt_pipe_crc_t *pipe_crc = data->pipe_crc;
igt_crc_t crc, ref_crc;
igt_plane_t *cursor;
cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
@@ -108,93 +101,93 @@ static void do_single_test(test_data_t *test_data, int x, int y)
igt_info("."); fflush(stdout);
/* Hardware test */
- igt_paint_test_pattern(cr, test_data->screenw, test_data->screenh);
- cursor_enable(test_data);
- cursor = igt_output_get_plane(test_data->output, IGT_PLANE_CURSOR);
+ igt_paint_test_pattern(cr, data->screenw, data->screenh);
+ cursor_enable(data);
+ cursor = igt_output_get_plane(data->output, IGT_PLANE_CURSOR);
igt_plane_set_position(cursor, x, y);
igt_display_commit(display);
- igt_wait_for_vblank(data->drm_fd, test_data->pipe);
+ igt_wait_for_vblank(data->drm_fd, data->pipe);
igt_pipe_crc_collect_crc(pipe_crc, &crc);
- cursor_disable(test_data);
+ cursor_disable(data);
/* Now render the same in software and collect crc */
- draw_cursor(cr, x, y, test_data->curw);
+ draw_cursor(cr, x, y, data->curw);
igt_display_commit(display);
- igt_wait_for_vblank(data->drm_fd, test_data->pipe);
+ igt_wait_for_vblank(data->drm_fd, data->pipe);
igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
/* Clear screen afterwards */
- igt_paint_color(cr, 0, 0, test_data->screenw, test_data->screenh,
- 0.0, 0.0, 0.0);
+ igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
+ 0.0, 0.0, 0.0);
igt_assert(igt_crc_equal(&crc, &ref_crc));
}
-static void do_test(test_data_t *test_data,
+static void do_test(data_t *data,
int left, int right, int top, int bottom)
{
- do_single_test(test_data, left, top);
- do_single_test(test_data, right, top);
- do_single_test(test_data, right, bottom);
- do_single_test(test_data, left, bottom);
+ do_single_test(data, left, top);
+ do_single_test(data, right, top);
+ do_single_test(data, right, bottom);
+ do_single_test(data, left, bottom);
}
-static void test_crc_onscreen(test_data_t *test_data)
+static void test_crc_onscreen(data_t *data)
{
- int left = test_data->left;
- int right = test_data->right;
- int top = test_data->top;
- int bottom = test_data->bottom;
- int cursor_w = test_data->curw;
- int cursor_h = test_data->curh;
+ int left = data->left;
+ int right = data->right;
+ int top = data->top;
+ int bottom = data->bottom;
+ int cursor_w = data->curw;
+ int cursor_h = data->curh;
/* fully inside */
- do_test(test_data, left, right, top, bottom);
+ do_test(data, left, right, top, bottom);
/* 2 pixels inside */
- do_test(test_data, left - (cursor_w-2), right + (cursor_w-2), top , bottom );
- do_test(test_data, left , right , top - (cursor_h-2), bottom + (cursor_h-2));
- do_test(test_data, left - (cursor_w-2), right + (cursor_w-2), top - (cursor_h-2), bottom + (cursor_h-2));
+ do_test(data, left - (cursor_w-2), right + (cursor_w-2), top , bottom );
+ do_test(data, left , right , top - (cursor_h-2), bottom + (cursor_h-2));
+ do_test(data, left - (cursor_w-2), right + (cursor_w-2), top - (cursor_h-2), bottom + (cursor_h-2));
/* 1 pixel inside */
- do_test(test_data, left - (cursor_w-1), right + (cursor_w-1), top , bottom );
- do_test(test_data, left , right , top - (cursor_h-1), bottom + (cursor_h-1));
- do_test(test_data, left - (cursor_w-1), right + (cursor_w-1), top - (cursor_h-1), bottom + (cursor_h-1));
+ do_test(data, left - (cursor_w-1), right + (cursor_w-1), top , bottom );
+ do_test(data, left , right , top - (cursor_h-1), bottom + (cursor_h-1));
+ do_test(data, left - (cursor_w-1), right + (cursor_w-1), top - (cursor_h-1), bottom + (cursor_h-1));
}
-static void test_crc_offscreen(test_data_t *test_data)
+static void test_crc_offscreen(data_t *data)
{
- int left = test_data->left;
- int right = test_data->right;
- int top = test_data->top;
- int bottom = test_data->bottom;
- int cursor_w = test_data->curw;
- int cursor_h = test_data->curh;
+ int left = data->left;
+ int right = data->right;
+ int top = data->top;
+ int bottom = data->bottom;
+ int cursor_w = data->curw;
+ int cursor_h = data->curh;
/* fully outside */
- do_test(test_data, left - (cursor_w), right + (cursor_w), top , bottom );
- do_test(test_data, left , right , top - (cursor_h), bottom + (cursor_h));
- do_test(test_data, left - (cursor_w), right + (cursor_w), top - (cursor_h), bottom + (cursor_h));
+ do_test(data, left - (cursor_w), right + (cursor_w), top , bottom );
+ do_test(data, left , right , top - (cursor_h), bottom + (cursor_h));
+ do_test(data, left - (cursor_w), right + (cursor_w), top - (cursor_h), bottom + (cursor_h));
/* fully outside by 1 extra pixels */
- do_test(test_data, left - (cursor_w+1), right + (cursor_w+1), top , bottom );
- do_test(test_data, left , right , top - (cursor_h+1), bottom + (cursor_h+1));
- do_test(test_data, left - (cursor_w+1), right + (cursor_w+1), top - (cursor_h+1), bottom + (cursor_h+1));
+ do_test(data, left - (cursor_w+1), right + (cursor_w+1), top , bottom );
+ do_test(data, left , right , top - (cursor_h+1), bottom + (cursor_h+1));
+ do_test(data, left - (cursor_w+1), right + (cursor_w+1), top - (cursor_h+1), bottom + (cursor_h+1));
/* fully outside by 2 extra pixels */
- do_test(test_data, left - (cursor_w+2), right + (cursor_w+2), top , bottom );
- do_test(test_data, left , right , top - (cursor_h+2), bottom + (cursor_h+2));
- do_test(test_data, left - (cursor_w+2), right + (cursor_w+2), top - (cursor_h+2), bottom + (cursor_h+2));
+ do_test(data, left - (cursor_w+2), right + (cursor_w+2), top , bottom );
+ do_test(data, left , right , top - (cursor_h+2), bottom + (cursor_h+2));
+ do_test(data, left - (cursor_w+2), right + (cursor_w+2), top - (cursor_h+2), bottom + (cursor_h+2));
/* fully outside by a lot of extra pixels */
- do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top , bottom );
- do_test(test_data, left , right , top - (cursor_h+512), bottom + (cursor_h+512));
- do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512));
+ do_test(data, left - (cursor_w+512), right + (cursor_w+512), top , bottom );
+ do_test(data, left , right , top - (cursor_h+512), bottom + (cursor_h+512));
+ do_test(data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512));
/* go nuts */
- do_test(test_data, INT_MIN, INT_MAX, INT_MIN, INT_MAX);
+ do_test(data, INT_MIN, INT_MAX, INT_MIN, INT_MAX);
}
-static void test_crc_sliding(test_data_t *test_data)
+static void test_crc_sliding(data_t *data)
{
int i;
@@ -202,34 +195,33 @@ static void test_crc_sliding(test_data_t *test_data)
* no alignment issues. Horizontal, vertical and diagonal test.
*/
for (i = 0; i < 16; i++) {
- do_single_test(test_data, i, 0);
- do_single_test(test_data, 0, i);
- do_single_test(test_data, i, i);
+ do_single_test(data, i, 0);
+ do_single_test(data, 0, i);
+ do_single_test(data, i, i);
}
}
-static void test_crc_random(test_data_t *test_data)
+static void test_crc_random(data_t *data)
{
int i;
/* Random cursor placement */
for (i = 0; i < 50; i++) {
- int x = rand() % (test_data->screenw + test_data->curw * 2) - test_data->curw;
- int y = rand() % (test_data->screenh + test_data->curh * 2) - test_data->curh;
- do_single_test(test_data, x, y);
+ int x = rand() % (data->screenw + data->curw * 2) - data->curw;
+ int y = rand() % (data->screenh + data->curh * 2) - data->curh;
+ do_single_test(data, x, y);
}
}
-static bool prepare_crtc(test_data_t *test_data, igt_output_t *output,
+static bool prepare_crtc(data_t *data, igt_output_t *output,
int cursor_w, int cursor_h)
{
drmModeModeInfo *mode;
- data_t *data = test_data->data;
igt_display_t *display = &data->display;
igt_plane_t *primary;
/* select the pipe we want to use */
- igt_output_set_pipe(output, test_data->pipe);
+ igt_output_set_pipe(output, data->pipe);
igt_display_commit(display);
if (!output->valid) {
@@ -241,10 +233,10 @@ static bool prepare_crtc(test_data_t *test_data, igt_output_t *output,
/* create and set the primary plane fb */
mode = igt_output_get_mode(output);
igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
- DRM_FORMAT_XRGB8888,
- false, /* tiled */
- 0.0, 0.0, 0.0,
- &data->primary_fb);
+ DRM_FORMAT_XRGB8888,
+ false, /* tiled */
+ 0.0, 0.0, 0.0,
+ &data->primary_fb);
primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
igt_plane_set_fb(primary, &data->primary_fb);
@@ -252,45 +244,44 @@ static bool prepare_crtc(test_data_t *test_data, igt_output_t *output,
igt_display_commit(display);
/* create the pipe_crc object for this pipe */
- if (test_data->pipe_crc)
- igt_pipe_crc_free(test_data->pipe_crc);
+ if (data->pipe_crc)
+ igt_pipe_crc_free(data->pipe_crc);
- test_data->pipe_crc = igt_pipe_crc_new(test_data->pipe,
- INTEL_PIPE_CRC_SOURCE_AUTO);
- if (!test_data->pipe_crc) {
+ data->pipe_crc = igt_pipe_crc_new(data->pipe,
+ INTEL_PIPE_CRC_SOURCE_AUTO);
+ if (!data->pipe_crc) {
igt_info("auto crc not supported on this connector with pipe %i\n",
- test_data->pipe);
+ data->pipe);
return false;
}
/* x/y position where the cursor is still fully visible */
- test_data->left = 0;
- test_data->right = mode->hdisplay - cursor_w;
- test_data->top = 0;
- test_data->bottom = mode->vdisplay - cursor_h;
- test_data->screenw = mode->hdisplay;
- test_data->screenh = mode->vdisplay;
- test_data->curw = cursor_w;
- test_data->curh = cursor_h;
+ data->left = 0;
+ data->right = mode->hdisplay - cursor_w;
+ data->top = 0;
+ data->bottom = mode->vdisplay - cursor_h;
+ data->screenw = mode->hdisplay;
+ data->screenh = mode->vdisplay;
+ data->curw = cursor_w;
+ data->curh = cursor_h;
/* make sure cursor is disabled */
- cursor_disable(test_data);
- igt_wait_for_vblank(data->drm_fd, test_data->pipe);
+ cursor_disable(data);
+ igt_wait_for_vblank(data->drm_fd, data->pipe);
/* get reference crc w/o cursor */
- igt_pipe_crc_collect_crc(test_data->pipe_crc, &test_data->ref_crc);
+ igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
return true;
}
-static void cleanup_crtc(test_data_t *test_data, igt_output_t *output)
+static void cleanup_crtc(data_t *data, igt_output_t *output)
{
- data_t *data = test_data->data;
igt_display_t *display = &data->display;
igt_plane_t *primary;
- igt_pipe_crc_free(test_data->pipe_crc);
- test_data->pipe_crc = NULL;
+ igt_pipe_crc_free(data->pipe_crc);
+ data->pipe_crc = NULL;
igt_remove_fb(data->drm_fd, &data->primary_fb);
@@ -301,38 +292,35 @@ static void cleanup_crtc(test_data_t *test_data, igt_output_t *output)
igt_display_commit(display);
}
-static void run_test(data_t *data, void (*testfunc)(test_data_t *), int cursor_w, int cursor_h)
+static void run_test(data_t *data, void (*testfunc)(data_t *), int cursor_w, int cursor_h)
{
igt_display_t *display = &data->display;
igt_output_t *output;
enum pipe p;
- test_data_t test_data = {
- .data = data,
- };
int valid_tests = 0;
for_each_connected_output(display, output) {
- test_data.output = output;
+ data->output = output;
for (p = 0; p < igt_display_get_n_pipes(display); p++) {
- test_data.pipe = p;
+ data->pipe = p;
- if (!prepare_crtc(&test_data, output, cursor_w, cursor_h))
+ if (!prepare_crtc(data, output, cursor_w, cursor_h))
continue;
valid_tests++;
igt_info("Beginning %s on pipe %c, connector %s\n",
- igt_subtest_name(), pipe_name(test_data.pipe),
+ igt_subtest_name(), pipe_name(data->pipe),
igt_output_name(output));
- testfunc(&test_data);
+ testfunc(data);
igt_info("\n%s on pipe %c, connector %s: PASSED\n\n",
- igt_subtest_name(), pipe_name(test_data.pipe),
+ igt_subtest_name(), pipe_name(data->pipe),
igt_output_name(output));
/* cleanup what prepare_crtc() has done */
- cleanup_crtc(&test_data, output);
+ cleanup_crtc(data, output);
}
}
--
1.8.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t 1/5] kms: Add universal plane support
2014-05-29 15:09 ` [PATCH i-g-t 1/5] kms: Add universal plane support Matt Roper
` (3 preceding siblings ...)
2014-05-29 15:09 ` [PATCH i-g-t 5/5] kms_cursor_crc: Combine data_t and test_data_t Matt Roper
@ 2014-06-13 16:21 ` Damien Lespiau
2014-06-13 17:00 ` Damien Lespiau
5 siblings, 0 replies; 14+ messages in thread
From: Damien Lespiau @ 2014-06-13 16:21 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Thu, May 29, 2014 at 08:09:13AM -0700, Matt Roper wrote:
> Add support for universal planes. This involves revamping the existing
> plane handling a bit to allow primary & cursor planes to come from the
> DRM plane list, rather than always being manually added. Also,
> eliminate the hard-coded position of primary & cursor in the plane list
> since the DRM could return them in any order.
>
> To minimize impact for existing tests, we add a new
> igt_display_use_universal_commits() call to toggle between universal and
> legacy API's for programming the primary and cursor planes.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
With the tiny change below, that patch looks reasonable. So:
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
One thing we may want to clean up when we bring in even more ways to do
commits is how we store the states to flush. What I mean by that is:
- In the *_set_*() functions (eg. igt_plane_set_fb()), we store the
states that change (eg. in set_fb() we only set fb_changed to true and
don't touch need_set_crtc/need_set_cursor)
- In the commit, we resolve what we need to do depending on the states
changed and the commit method.
> ---
> lib/igt_kms.c | 132 +++++++++++++++++++++++++++++++++++++++++++++-------------
> lib/igt_kms.h | 5 +++
> 2 files changed, 107 insertions(+), 30 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index d00250d..97e329b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -500,27 +500,24 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> */
> display->n_pipes = resources->count_crtcs;
>
> + drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
Can we have:
#ifndef DRM_CLIENT_CAP_UNIVERSAL_PLANES
#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2
#endif
As an easy way to make sure that's always defined, even if we don't have
the (unreleased) libdrm with this support?
> plane_resources = drmModeGetPlaneResources(display->drm_fd);
> igt_assert(plane_resources);
>
> for (i = 0; i < display->n_pipes; i++) {
> igt_pipe_t *pipe = &display->pipes[i];
> - igt_plane_t *plane;
> - int p, j;
> + igt_plane_t *plane = NULL;
> + int p = 0, j;
>
> pipe->display = display;
> pipe->pipe = i;
>
> - /* primary plane */
> - p = IGT_PLANE_PRIMARY;
> - plane = &pipe->planes[p];
> - plane->pipe = pipe;
> - plane->index = p;
> - plane->is_primary = true;
> -
> /* add the planes that can be used with that pipe */
> for (j = 0; j < plane_resources->count_planes; j++) {
> drmModePlane *drm_plane;
> + drmModeObjectPropertiesPtr proplist;
> + drmModePropertyPtr prop;
> + int k;
>
> drm_plane = drmModeGetPlane(display->drm_fd,
> plane_resources->planes[j]);
> @@ -531,21 +528,67 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> continue;
> }
>
> - p++;
> plane = &pipe->planes[p];
> plane->pipe = pipe;
> - plane->index = p;
> + plane->index = p++;
> plane->drm_plane = drm_plane;
> +
> + prop = NULL;
> + proplist = drmModeObjectGetProperties(display->drm_fd,
> + plane_resources->planes[j],
> + DRM_MODE_OBJECT_PLANE);
> + for (k = 0; k < proplist->count_props; k++) {
> + prop = drmModeGetProperty(display->drm_fd, proplist->props[k]);
> + if (!prop)
> + continue;
> +
> + if (strcmp(prop->name, "type") != 0) {
> + drmModeFreeProperty(prop);
> + continue;
> + }
> +
> + switch (proplist->prop_values[k]) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + plane->is_primary = 1;
> + display->has_universal_planes = 1;
> + break;
> + case DRM_PLANE_TYPE_CURSOR:
> + plane->is_cursor = 1;
> + pipe->has_cursor = 1;
> + display->has_universal_planes = 1;
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + break;
> + }
> +
> }
>
> - /* cursor plane */
> - p++;
> - plane = &pipe->planes[p];
> - plane->pipe = pipe;
> - plane->index = p;
> - plane->is_cursor = true;
> + /*
> + * Add a drm_plane-less primary plane for kernels without
> + * universal plane support.
> + */
> + if (!display->has_universal_planes) {
> + plane = &pipe->planes[p];
> + plane->pipe = pipe;
> + plane->index = p++;
> + plane->is_primary = true;
> + }
>
> - pipe->n_planes = ++p;
> + /*
> + * Add drm_plane-less cursor plane for kernels that don't
> + * expose a universal cursor plane.
> + */
> + if (!pipe->has_cursor) {
> + /* cursor plane */
> + plane = &pipe->planes[p];
> + plane->pipe = pipe;
> + plane->index = p++;
> + plane->is_cursor = true;
> + }
> +
> + pipe->n_planes = p;
>
> /* make sure we don't overflow the plane array */
> igt_assert(pipe->n_planes <= IGT_MAX_PLANES);
> @@ -700,16 +743,29 @@ static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, enum igt_plane plane)
> {
> int idx;
>
> - /* Cursor plane is always the upper plane */
> - if (plane == IGT_PLANE_CURSOR)
> - idx = pipe->n_planes - 1;
> - else {
> - igt_assert_f(plane >= 0 && plane < (pipe->n_planes),
> - "plane=%d\n", plane);
> - idx = plane;
> + for (idx = 0; idx < pipe->n_planes; idx++) {
> + switch (plane) {
> + case IGT_PLANE_PRIMARY:
> + if (pipe->planes[idx].is_primary)
> + return &pipe->planes[idx];
> + break;
> + case IGT_PLANE_CURSOR:
> + if (pipe->planes[idx].is_cursor)
> + return &pipe->planes[idx];
> + break;
> + case IGT_PLANE_2:
> + if (!pipe->planes[idx].is_primary &&
> + !pipe->planes[idx].is_cursor)
> + return &pipe->planes[idx];
> + default:
> + if (!pipe->planes[idx].is_primary &&
> + !pipe->planes[idx].is_cursor)
> + /* Consume this overlay and keep searching for another */
> + plane--;
> + }
> }
>
> - return &pipe->planes[idx];
> + return NULL;
> }
>
> static uint32_t igt_plane_get_fb_id(igt_plane_t *plane)
> @@ -761,6 +817,8 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
> uint32_t fb_id, crtc_id;
> int ret;
>
> + igt_assert(plane->drm_plane);
> +
> fb_id = igt_plane_get_fb_id(plane);
> crtc_id = output->config.crtc->crtc_id;
> pipe = igt_output_get_driving_pipe(output);
> @@ -820,7 +878,11 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
>
> static int igt_plane_commit(igt_plane_t *plane, igt_output_t *output)
> {
> - if (plane->is_cursor) {
> + struct igt_display *display = plane->pipe->display;
> +
> + if (display->commit_universal && plane->drm_plane) {
> + igt_drm_plane_commit(plane, output);
> + } else if (plane->is_cursor) {
> igt_cursor_commit(plane, output);
> } else if (plane->is_primary) {
> /* state updated by SetCrtc */
> @@ -838,8 +900,8 @@ static int igt_output_commit(igt_output_t *output)
> int i;
>
> pipe = igt_output_get_driving_pipe(output);
> - if (pipe->need_set_crtc) {
> - igt_plane_t *primary = &pipe->planes[0];
> + if (pipe->need_set_crtc && !display->commit_universal) {
> + igt_plane_t *primary = igt_pipe_get_plane(pipe, IGT_PLANE_PRIMARY);
> drmModeModeInfo *mode;
> uint32_t fb_id, crtc_id;
> int ret;
> @@ -889,7 +951,7 @@ static int igt_output_commit(igt_output_t *output)
> primary->fb_changed = false;
> }
>
> - if (pipe->need_set_cursor) {
> + if (pipe->need_set_cursor && !display->commit_universal) {
> igt_plane_t *cursor;
> uint32_t gem_handle, crtc_id;
> int ret;
> @@ -940,6 +1002,16 @@ static int igt_output_commit(igt_output_t *output)
> return 0;
> }
>
> +/*
> + * Indicate whether future display commits should use universal plane API's
> + * or legacy API's.
> + */
> +void igt_display_use_universal_commits(igt_display_t *display,
> + bool use_universal)
> +{
> + display->commit_universal = use_universal;
> +}
> +
> int igt_display_commit(igt_display_t *display)
> {
> int i;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 8e80d4b..4955fc8 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -121,6 +121,7 @@ struct igt_pipe {
> unsigned int need_set_crtc : 1;
> unsigned int need_set_cursor : 1;
> unsigned int need_wait_for_vblank : 1;
> + unsigned int has_cursor : 1;
> #define IGT_MAX_PLANES 4
> int n_planes;
> igt_plane_t planes[IGT_MAX_PLANES];
> @@ -143,6 +144,8 @@ struct igt_display {
> unsigned long pipes_in_use;
> igt_output_t *outputs;
> igt_pipe_t pipes[I915_MAX_PIPES];
> + bool has_universal_planes;
> + bool commit_universal;
> };
>
> /* set vt into graphics mode, required to prevent fbcon from interfering */
> @@ -150,6 +153,8 @@ void igt_set_vt_graphics_mode(void);
>
> void igt_display_init(igt_display_t *display, int drm_fd);
> void igt_display_fini(igt_display_t *display);
> +void igt_display_use_universal_commits(igt_display_t *display,
> + bool use_universal);
> int igt_display_commit(igt_display_t *display);
> int igt_display_get_n_pipes(igt_display_t *display);
>
> --
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t 2/5] kms: Add igt_drm_plane_try_commit()
2014-05-29 15:09 ` [PATCH i-g-t 2/5] kms: Add igt_drm_plane_try_commit() Matt Roper
@ 2014-06-13 16:24 ` Damien Lespiau
0 siblings, 0 replies; 14+ messages in thread
From: Damien Lespiau @ 2014-06-13 16:24 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Thu, May 29, 2014 at 08:09:14AM -0700, Matt Roper wrote:
> For some of our tests, we want to make sure that bogus plane programming
> attempts fail with the expected error code. Add
> igt_drm_plane_try_commit() that will just return the error code on
> failure rather than failing the current subtest. This lets us test the
> return value against the expected error code.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Huum, I was careful not to expose any of the sub-commit functions,
because of atomic modeset.
I don't think we want to expose the commit of a single plane here, so
it'd have to be igt_display_try_commit(). That should be equivalent for
your need but hopefully integrate with the atomic modeset ioctl() as
well.
--
Damien
> ---
> lib/igt_kms.c | 21 ++++++++++++++++++---
> lib/igt_kms.h | 1 +
> 2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 97e329b..ce6ea87 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -810,7 +810,11 @@ static int igt_cursor_commit(igt_plane_t *plane, igt_output_t *output)
> return 0;
> }
>
> -static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
> +/*
> + * Attempt to commit a plane; if the DRM call to program the plane fails,
> + * just return an error code (but don't fail the current subtest).
> + */
> +int igt_drm_plane_try_commit(igt_plane_t *plane, igt_output_t *output)
> {
> igt_display_t *display = output->display;
> igt_pipe_t *pipe;
> @@ -842,7 +846,8 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
> IGT_FIXED(0,0), /* src_w */
> IGT_FIXED(0,0) /* src_h */);
>
> - igt_assert(ret == 0);
> + if (ret)
> + return ret;
>
> plane->fb_changed = false;
> } else if (plane->fb_changed || plane->position_changed) {
> @@ -866,7 +871,8 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
> IGT_FIXED(plane->fb->width,0), /* src_w */
> IGT_FIXED(plane->fb->height,0) /* src_h */);
>
> - igt_assert(ret == 0);
> + if (ret)
> + return ret;
>
> plane->fb_changed = false;
> plane->position_changed = false;
> @@ -876,6 +882,15 @@ static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
> return 0;
> }
>
> +static int igt_drm_plane_commit(igt_plane_t *plane, igt_output_t *output)
> +{
> + int ret = igt_drm_plane_try_commit(plane, output);
> +
> + igt_assert(ret == 0);
> +
> + return 0;
> +}
> +
> static int igt_plane_commit(igt_plane_t *plane, igt_output_t *output)
> {
> struct igt_display *display = plane->pipe->display;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 4955fc8..2f72a1c 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -157,6 +157,7 @@ void igt_display_use_universal_commits(igt_display_t *display,
> bool use_universal);
> int igt_display_commit(igt_display_t *display);
> int igt_display_get_n_pipes(igt_display_t *display);
> +int igt_drm_plane_try_commit(igt_plane_t *plane, igt_output_t *output);
>
> const char *igt_output_name(igt_output_t *output);
> drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
> --
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t 3/5] kms_plane: Update for universal plane changes
2014-05-29 15:09 ` [PATCH i-g-t 3/5] kms_plane: Update for universal plane changes Matt Roper
@ 2014-06-13 16:27 ` Damien Lespiau
0 siblings, 0 replies; 14+ messages in thread
From: Damien Lespiau @ 2014-06-13 16:27 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Thu, May 29, 2014 at 08:09:15AM -0700, Matt Roper wrote:
> Recent changes in igt_kms to support universal planes have removed the
> plane list order guarantees that kms_plane depended upon. Ensure that
> we loop over the entire plane list properly and then selectively skip
> non-overlay planes.
>
> While we're at it, update a couple igt_output_get_plane() calls to use
> plane enums rather than integer values to make it clear what we're
> actually doing.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
I've hard time deciding if I'm happy with losing the ordering of planes.
I think I'm not :) Can we have the init function keep the plane
ordering?
--
Damien
> ---
> tests/kms_plane.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 5db0947..1d334ab 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -94,7 +94,7 @@ test_position_init(test_position_t *test, igt_output_t *output, enum pipe pipe)
> test->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
>
> igt_output_set_pipe(output, pipe);
> - primary = igt_output_get_plane(output, 0);
> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
>
> mode = igt_output_get_mode(output);
> igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> @@ -146,9 +146,14 @@ test_plane_position_with_output(data_t *data,
> test_position_init(&test, output, pipe);
>
> mode = igt_output_get_mode(output);
> - primary = igt_output_get_plane(output, 0);
> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> sprite = igt_output_get_plane(output, plane);
>
> + if (sprite->is_primary) {
> + test_position_fini(&test, output);
> + igt_skip("Skipping primary plane\n");
> + }
> +
> create_fb_for_mode__position(data, mode, 100, 100, 64, 64,
> &primary_fb);
> igt_plane_set_fb(primary, &primary_fb);
> @@ -213,7 +218,7 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
> {
> int plane;
>
> - for (plane = 1; plane < IGT_MAX_PLANES; plane++)
> + for (plane = 0; plane < IGT_MAX_PLANES; plane++)
> run_tests_for_pipe_plane(data, pipe, plane);
> }
>
> --
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t 1/5] kms: Add universal plane support
2014-05-29 15:09 ` [PATCH i-g-t 1/5] kms: Add universal plane support Matt Roper
` (4 preceding siblings ...)
2014-06-13 16:21 ` [PATCH i-g-t 1/5] kms: Add universal plane support Damien Lespiau
@ 2014-06-13 17:00 ` Damien Lespiau
5 siblings, 0 replies; 14+ messages in thread
From: Damien Lespiau @ 2014-06-13 17:00 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
As a small summary, I don't think we should be exposing the
plane_commit() function and always go through igt_display_commit() to
make hooking the atomic ioctl() easier. Loosing the plane ordering is a
bit meh but should be able to live with it. The rest looks good.
--
Damien
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-06-13 17:00 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-29 15:06 [PATCH 0/4] Intel primary plane support (v7) Matt Roper
2014-05-29 15:06 ` [PATCH 1/4] drm: Check CRTC compatibility in setplane Matt Roper
2014-05-29 15:06 ` [PATCH 2/4] drm/plane-helper: Add drm_plane_helper_check_update() (v3) Matt Roper
2014-05-29 15:06 ` [PATCH 3/4] drm/i915: don't force full modeset if primary plane is disabled (v2) Matt Roper
2014-05-29 15:06 ` [PATCH 4/4] drm/i915: Intel-specific primary plane handling (v8) Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 1/5] kms: Add universal plane support Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 2/5] kms: Add igt_drm_plane_try_commit() Matt Roper
2014-06-13 16:24 ` Damien Lespiau
2014-05-29 15:09 ` [PATCH i-g-t 3/5] kms_plane: Update for universal plane changes Matt Roper
2014-06-13 16:27 ` Damien Lespiau
2014-05-29 15:09 ` [PATCH i-g-t 4/5] kms_universal_plane: Universal plane testing (v5) Matt Roper
2014-05-29 15:09 ` [PATCH i-g-t 5/5] kms_cursor_crc: Combine data_t and test_data_t Matt Roper
2014-06-13 16:21 ` [PATCH i-g-t 1/5] kms: Add universal plane support Damien Lespiau
2014-06-13 17:00 ` Damien Lespiau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.