* [PATCH 01/15] drm: Add drm_rotation_90_or_270()
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 13:43 ` [PATCH 02/15] drm/atomic: Reject attempts to use multiple rotation angles at once ville.syrjala
` (15 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We have intel_rotation_90_or_270() in i915 to check if the rotation is
90 or 270 degrees. Similar checks are elsewhere in drm, so let's move
the helper into a central place and use it everwhere.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 4 ++--
drivers/gpu/drm/drm_atomic_helper.c | 2 +-
drivers/gpu/drm/drm_crtc.c | 3 +--
drivers/gpu/drm/i915/intel_atomic_plane.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
drivers/gpu/drm/i915/intel_drv.h | 6 ------
drivers/gpu/drm/i915/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 12 ++++++------
drivers/gpu/drm/i915/intel_sprite.c | 2 +-
drivers/gpu/drm/omapdrm/omap_plane.c | 8 ++------
include/drm/drm_crtc.h | 5 +++++
11 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 016c191221f3..f3350c80704d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -393,7 +393,7 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
if ((state->base.fb->pixel_format == DRM_FORMAT_YUV422 ||
state->base.fb->pixel_format == DRM_FORMAT_NV61) &&
- (state->base.rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))))
+ drm_rotation_90_or_270(state->base.rotation))
cfg |= ATMEL_HLCDC_YUV422ROT;
atmel_hlcdc_layer_update_cfg(&plane->layer,
@@ -628,7 +628,7 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
/*
* Swap width and size in case of 90 or 270 degrees rotation
*/
- if (state->base.rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {
+ if (drm_rotation_90_or_270(state->base.rotation)) {
tmp = state->crtc_w;
state->crtc_w = state->crtc_h;
state->crtc_h = tmp;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index de7fddce3cef..3e12e6e646ad 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2348,7 +2348,7 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
primary_state->crtc_h = vdisplay;
primary_state->src_x = set->x << 16;
primary_state->src_y = set->y << 16;
- if (primary_state->rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {
+ if (drm_rotation_90_or_270(primary_state->rotation)) {
primary_state->src_w = vdisplay << 16;
primary_state->src_h = hdisplay << 16;
} else {
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f1d9f0569d7f..986de180e6c2 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2804,8 +2804,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
if (crtc->state &&
- crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) |
- BIT(DRM_ROTATE_270)))
+ drm_rotation_90_or_270(crtc->primary->state->rotation))
swap(hdisplay, vdisplay);
return check_src_coords(x << 16, y << 16,
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 7de7721f65bc..c19eb9a0cd4a 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -156,7 +156,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
intel_state->clip.y2 =
crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
- if (state->fb && intel_rotation_90_or_270(state->rotation)) {
+ if (state->fb && drm_rotation_90_or_270(state->rotation)) {
if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 78beb7e9d384..51357986cf4a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2138,7 +2138,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
const struct drm_framebuffer *fb,
unsigned int rotation)
{
- if (intel_rotation_90_or_270(rotation)) {
+ if (drm_rotation_90_or_270(rotation)) {
*view = i915_ggtt_view_rotated;
view->params.rotated = to_intel_framebuffer(fb)->rot_info;
} else {
@@ -2352,7 +2352,7 @@ u32 intel_compute_tile_offset(int *x, int *y,
intel_tile_dims(dev_priv, &tile_width, &tile_height,
fb_modifier, cpp);
- if (intel_rotation_90_or_270(rotation)) {
+ if (drm_rotation_90_or_270(rotation)) {
pitch_tiles = pitch / tile_height;
swap(tile_width, tile_height);
} else {
@@ -3011,7 +3011,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
WARN_ON(drm_rect_width(&plane_state->src) == 0);
- if (intel_rotation_90_or_270(rotation)) {
+ if (drm_rotation_90_or_270(rotation)) {
int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
/* stride = Surface height in tiles */
@@ -4179,7 +4179,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
to_intel_crtc(crtc_state->base.crtc);
int need_scaling;
- need_scaling = intel_rotation_90_or_270(rotation) ?
+ need_scaling = drm_rotation_90_or_270(rotation) ?
(src_h != dst_w || src_w != dst_h):
(src_w != dst_w || src_h != dst_h);
@@ -11413,7 +11413,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
* The stride is either expressed as a multiple of 64 bytes chunks for
* linear buffers or in number of tiles for tiled buffers.
*/
- if (intel_rotation_90_or_270(rotation)) {
+ if (drm_rotation_90_or_270(rotation)) {
/* stride = Surface height in tiles */
tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);
stride = DIV_ROUND_UP(fb->height, tile_height);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e74d851868c5..907a72cfdad3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1255,12 +1255,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
uint64_t fb_modifier, unsigned int cpp);
-static inline bool
-intel_rotation_90_or_270(unsigned int rotation)
-{
- return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));
-}
-
void intel_create_rotation_property(struct drm_device *dev,
struct intel_plane *plane);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 781e2f5f7cd8..f3f3322139e0 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -84,7 +84,7 @@ static void intel_fbc_get_plane_source_size(struct intel_fbc_state_cache *cache,
{
int w, h;
- if (intel_rotation_90_or_270(cache->plane.rotation)) {
+ if (drm_rotation_90_or_270(cache->plane.rotation)) {
w = cache->plane.src_h;
h = cache->plane.src_w;
} else {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 64d628c915a3..3a1873ed26b3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3004,7 +3004,7 @@ skl_plane_downscale_amount(const struct intel_plane_state *pstate)
src_h = drm_rect_height(&pstate->src);
dst_w = drm_rect_width(&pstate->dst);
dst_h = drm_rect_height(&pstate->dst);
- if (intel_rotation_90_or_270(pstate->base.rotation))
+ if (drm_rotation_90_or_270(pstate->base.rotation))
swap(dst_w, dst_h);
downscale_h = max(src_h / dst_h, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
@@ -3035,7 +3035,7 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
width = drm_rect_width(&intel_pstate->src) >> 16;
height = drm_rect_height(&intel_pstate->src) >> 16;
- if (intel_rotation_90_or_270(pstate->rotation))
+ if (drm_rotation_90_or_270(pstate->rotation))
swap(width, height);
/* for planar format */
@@ -3137,7 +3137,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
src_w = drm_rect_width(&intel_pstate->src) >> 16;
src_h = drm_rect_height(&intel_pstate->src) >> 16;
- if (intel_rotation_90_or_270(pstate->rotation))
+ if (drm_rotation_90_or_270(pstate->rotation))
swap(src_w, src_h);
/* Halve UV plane width and height for NV12 */
@@ -3151,7 +3151,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
else
plane_bpp = drm_format_plane_cpp(fb->pixel_format, 0);
- if (intel_rotation_90_or_270(pstate->rotation)) {
+ if (drm_rotation_90_or_270(pstate->rotation)) {
switch (plane_bpp) {
case 1:
min_scanlines = 32;
@@ -3407,7 +3407,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
width = drm_rect_width(&intel_pstate->src) >> 16;
height = drm_rect_height(&intel_pstate->src) >> 16;
- if (intel_rotation_90_or_270(pstate->rotation))
+ if (drm_rotation_90_or_270(pstate->rotation))
swap(width, height);
cpp = drm_format_plane_cpp(fb->pixel_format, 0);
@@ -3428,7 +3428,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
uint32_t min_scanlines = 4;
uint32_t y_tile_minimum;
- if (intel_rotation_90_or_270(pstate->rotation)) {
+ if (drm_rotation_90_or_270(pstate->rotation)) {
int cpp = (fb->pixel_format == DRM_FORMAT_NV12) ?
drm_format_plane_cpp(fb->pixel_format, 1) :
drm_format_plane_cpp(fb->pixel_format, 0);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0de935ad01c2..cc6af1410d67 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -251,7 +251,7 @@ skl_update_plane(struct drm_plane *drm_plane,
surf_addr = intel_plane_obj_offset(intel_plane, obj, 0);
- if (intel_rotation_90_or_270(rotation)) {
+ if (drm_rotation_90_or_270(rotation)) {
int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
/* stride: Surface height in tiles */
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 5252ab720e70..7f0d567b8d67 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -108,16 +108,12 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
win.src_x = state->src_x >> 16;
win.src_y = state->src_y >> 16;
- switch (state->rotation & DRM_ROTATE_MASK) {
- case BIT(DRM_ROTATE_90):
- case BIT(DRM_ROTATE_270):
+ if (drm_rotation_90_or_270(state->rotation)) {
win.src_w = state->src_h >> 16;
win.src_h = state->src_w >> 16;
- break;
- default:
+ } else {
win.src_w = state->src_w >> 16;
win.src_h = state->src_h >> 16;
- break;
}
/* update scanout: */
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3edeaf88ebc0..b7ac7dcf52db 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -92,6 +92,11 @@ static inline uint64_t I642U64(int64_t val)
#define DRM_REFLECT_X 4
#define DRM_REFLECT_Y 5
+static inline bool drm_rotation_90_or_270(unsigned int rotation)
+{
+ return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));
+}
+
enum drm_connector_force {
DRM_FORCE_UNSPECIFIED,
DRM_FORCE_OFF,
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 02/15] drm/atomic: Reject attempts to use multiple rotation angles at once
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
2016-07-22 13:43 ` [PATCH 01/15] drm: Add drm_rotation_90_or_270() ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 13:43 ` [PATCH v2 03/15] drm: Add support for optional per-plane rotation property ville.syrjala
` (14 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The rotation property should only accept exactly one rotation angle
at once. Let's reject attempts to set none or multiple angles.
Testcase: igt/kms_rotation_crc/bad-rotation
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/drm_atomic.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8d2f111fa113..c425bd39a210 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -710,6 +710,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
} else if (property == config->prop_src_h) {
state->src_h = val;
} else if (property == config->rotation_property) {
+ if (!is_power_of_2(val & DRM_ROTATE_MASK))
+ return -EINVAL;
state->rotation = val;
} else if (plane->funcs->atomic_set_property) {
return plane->funcs->atomic_set_property(plane, state,
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v2 03/15] drm: Add support for optional per-plane rotation property
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
2016-07-22 13:43 ` [PATCH 01/15] drm: Add drm_rotation_90_or_270() ville.syrjala
2016-07-22 13:43 ` [PATCH 02/15] drm/atomic: Reject attempts to use multiple rotation angles at once ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-25 7:08 ` Joonas Lahtinen
2016-07-22 13:43 ` [PATCH 04/15] drm/arm: Use " ville.syrjala
` (13 subsequent siblings)
16 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Not all planes on the ssytem may support the same rotations/reflections,
so make it possible to create a separate property for each plane.
This way userspace gets told exactly which rotations/reflections are
possible for each plane.
v2: Add drm_plane_create_rotation_property() helper
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
---
drivers/gpu/drm/drm_atomic.c | 6 ++++--
drivers/gpu/drm/drm_crtc.c | 33 +++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_fb_helper.c | 6 +++++-
include/drm/drm_crtc.h | 10 +++++++++-
4 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index c425bd39a210..116f940a9267 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -709,7 +709,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
state->src_w = val;
} else if (property == config->prop_src_h) {
state->src_h = val;
- } else if (property == config->rotation_property) {
+ } else if (property == config->rotation_property ||
+ property == plane->rotation_property) {
if (!is_power_of_2(val & DRM_ROTATE_MASK))
return -EINVAL;
state->rotation = val;
@@ -767,7 +768,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_w;
} else if (property == config->prop_src_h) {
*val = state->src_h;
- } else if (property == config->rotation_property) {
+ } else if (property == config->rotation_property ||
+ property == plane->rotation_property) {
*val = state->rotation;
} else if (plane->funcs->atomic_get_property) {
return plane->funcs->atomic_get_property(plane, state, property, val);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 986de180e6c2..9e20a52ece7c 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5801,6 +5801,39 @@ struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_mode_create_rotation_property);
+int drm_plane_create_rotation_property(struct drm_plane *plane,
+ unsigned int rotation,
+ unsigned int supported_rotations)
+{
+ static const struct drm_prop_enum_list props[] = {
+ { DRM_ROTATE_0, "rotate-0" },
+ { DRM_ROTATE_90, "rotate-90" },
+ { DRM_ROTATE_180, "rotate-180" },
+ { DRM_ROTATE_270, "rotate-270" },
+ { DRM_REFLECT_X, "reflect-x" },
+ { DRM_REFLECT_Y, "reflect-y" },
+ };
+ struct drm_property *prop;
+
+ WARN_ON((supported_rotations & rotation) == 0);
+
+ prop = drm_property_create_bitmask(plane->dev, 0, "rotation",
+ props, ARRAY_SIZE(props),
+ supported_rotations);
+ if (!prop)
+ return -ENOMEM;
+
+ drm_object_attach_property(&plane->base, prop, rotation);
+
+ if (plane->state)
+ plane->state->rotation = rotation;
+
+ plane->rotation_property = prop;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_rotation_property);
+
/**
* DOC: Tile group
*
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index ce54e985d91b..ce536c0553e5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -392,7 +392,11 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
if (plane->type != DRM_PLANE_TYPE_PRIMARY)
drm_plane_force_disable(plane);
- if (dev->mode_config.rotation_property) {
+ if (plane->rotation_property) {
+ drm_mode_plane_set_obj_prop(plane,
+ plane->rotation_property,
+ BIT(DRM_ROTATE_0));
+ } else if (dev->mode_config.rotation_property) {
drm_mode_plane_set_obj_prop(plane,
dev->mode_config.rotation_property,
BIT(DRM_ROTATE_0));
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b7ac7dcf52db..01cf0673f6c8 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1737,6 +1737,11 @@ struct drm_plane {
const struct drm_plane_helper_funcs *helper_private;
struct drm_plane_state *state;
+
+ /**
+ * @rotation_property: Optional property for planes to specify rotation.
+ */
+ struct drm_property *rotation_property;
};
/**
@@ -2475,7 +2480,7 @@ struct drm_mode_config {
*/
struct drm_property *plane_type_property;
/**
- * @rotation_property: Optional property for planes or CRTCs to specifiy
+ * @rotation_property: Optional property for planes or CRTCs to specify
* rotation.
*/
struct drm_property *rotation_property;
@@ -2957,6 +2962,9 @@ extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
unsigned int supported_rotations);
+extern int drm_plane_create_rotation_property(struct drm_plane *plane,
+ unsigned int rotation,
+ unsigned int supported_rotations);
extern unsigned int drm_rotation_simplify(unsigned int rotation,
unsigned int supported_rotations);
extern void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v2 03/15] drm: Add support for optional per-plane rotation property
2016-07-22 13:43 ` [PATCH v2 03/15] drm: Add support for optional per-plane rotation property ville.syrjala
@ 2016-07-25 7:08 ` Joonas Lahtinen
0 siblings, 0 replies; 41+ messages in thread
From: Joonas Lahtinen @ 2016-07-25 7:08 UTC (permalink / raw)
To: ville.syrjala, dri-devel; +Cc: intel-gfx
On pe, 2016-07-22 at 16:43 +0300, ville.syrjala@linux.intel.com wrote:
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 986de180e6c2..9e20a52ece7c 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5801,6 +5801,39 @@ struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_mode_create_rotation_property);
>
> +int drm_plane_create_rotation_property(struct drm_plane *plane,
> + unsigned int rotation,
> + unsigned int supported_rotations)
> +{
> + static const struct drm_prop_enum_list props[] = {
> + { DRM_ROTATE_0, "rotate-0" },
> + { DRM_ROTATE_90, "rotate-90" },
> + { DRM_ROTATE_180, "rotate-180" },
> + { DRM_ROTATE_270, "rotate-270" },
> + { DRM_REFLECT_X, "reflect-x" },
> + { DRM_REFLECT_Y, "reflect-y" },
I sent a series for converting the DRM_ROTATE_? defines into BIT()
variants, so __builtin_ffs(DRM_ROTATE_0) - 1 etc. would end up used
here depending on which gets merged first.
Also,
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 04/15] drm/arm: Use per-plane rotation property
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (2 preceding siblings ...)
2016-07-22 13:43 ` [PATCH v2 03/15] drm: Add support for optional per-plane rotation property ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 14:37 ` Brian Starkey
2016-07-22 13:43 ` [PATCH 05/15] drm/atmel-hlcdc: " ville.syrjala
` (12 subsequent siblings)
16 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: Mali DP Maintainers, intel-gfx, Liviu Dudau, Brian Starkey
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The global mode_config.rotation_property is going away, switch over to
per-plane rotation_property.
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Brian Starkey <brian.starkey@arm.com>
Cc: Mali DP Maintainers <malidp@foss.arm.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/arm/malidp_planes.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 725098d6179a..7fbe1feec861 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -254,21 +254,18 @@ int malidp_de_planes_init(struct drm_device *drm)
if (ret < 0)
goto cleanup;
- if (!drm->mode_config.rotation_property) {
+ /* SMART layer can't be rotated */
+ if (id != DE_SMART) {
unsigned long flags = BIT(DRM_ROTATE_0) |
BIT(DRM_ROTATE_90) |
BIT(DRM_ROTATE_180) |
BIT(DRM_ROTATE_270) |
BIT(DRM_REFLECT_X) |
BIT(DRM_REFLECT_Y);
- drm->mode_config.rotation_property =
- drm_mode_create_rotation_property(drm, flags);
+ drm_plane_create_rotation_property(&plane->base,
+ BIT(DRM_ROTATE_0),
+ flags);
}
- /* SMART layer can't be rotated */
- if (drm->mode_config.rotation_property && (id != DE_SMART))
- drm_object_attach_property(&plane->base.base,
- drm->mode_config.rotation_property,
- BIT(DRM_ROTATE_0));
drm_plane_helper_add(&plane->base,
&malidp_de_plane_helper_funcs);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 04/15] drm/arm: Use per-plane rotation property
2016-07-22 13:43 ` [PATCH 04/15] drm/arm: Use " ville.syrjala
@ 2016-07-22 14:37 ` Brian Starkey
0 siblings, 0 replies; 41+ messages in thread
From: Brian Starkey @ 2016-07-22 14:37 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Liviu Dudau, dri-devel
On Fri, Jul 22, 2016 at 04:43:05PM +0300, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>The global mode_config.rotation_property is going away, switch over to
>per-plane rotation_property.
>
>Cc: Liviu Dudau <liviu.dudau@arm.com>
>Cc: Brian Starkey <brian.starkey@arm.com>
>Cc: Mali DP Maintainers <malidp@foss.arm.com>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Brian Starkey <brian.starkey@arm.com>
Thanks!
-Brian
>---
> drivers/gpu/drm/arm/malidp_planes.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
>index 725098d6179a..7fbe1feec861 100644
>--- a/drivers/gpu/drm/arm/malidp_planes.c
>+++ b/drivers/gpu/drm/arm/malidp_planes.c
>@@ -254,21 +254,18 @@ int malidp_de_planes_init(struct drm_device *drm)
> if (ret < 0)
> goto cleanup;
>
>- if (!drm->mode_config.rotation_property) {
>+ /* SMART layer can't be rotated */
>+ if (id != DE_SMART) {
> unsigned long flags = BIT(DRM_ROTATE_0) |
> BIT(DRM_ROTATE_90) |
> BIT(DRM_ROTATE_180) |
> BIT(DRM_ROTATE_270) |
> BIT(DRM_REFLECT_X) |
> BIT(DRM_REFLECT_Y);
>- drm->mode_config.rotation_property =
>- drm_mode_create_rotation_property(drm, flags);
>+ drm_plane_create_rotation_property(&plane->base,
>+ BIT(DRM_ROTATE_0),
>+ flags);
> }
>- /* SMART layer can't be rotated */
>- if (drm->mode_config.rotation_property && (id != DE_SMART))
>- drm_object_attach_property(&plane->base.base,
>- drm->mode_config.rotation_property,
>- BIT(DRM_ROTATE_0));
>
> drm_plane_helper_add(&plane->base,
> &malidp_de_plane_helper_funcs);
>--
>2.7.4
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (3 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 04/15] drm/arm: Use " ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 13:58 ` Boris Brezillon
2016-07-22 15:47 ` [PATCH v2 " ville.syrjala
2016-07-22 13:43 ` [PATCH 06/15] drm/omap: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0 ville.syrjala
` (11 subsequent siblings)
16 siblings, 2 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The global mode_config.rotation_property is going away, switch over to
per-plane rotation_property.
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index f3350c80704d..be3d4310ea97 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -903,9 +903,12 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
}
if (desc->layout.xstride && desc->layout.pstride)
- drm_object_attach_property(&plane->base.base,
- plane->base.dev->mode_config.rotation_property,
- BIT(DRM_ROTATE_0));
+ drm_plane_create_rotation_property(&plane->base,
+ BIT(DRM_ROTATE_0),
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_90) |
+ BIT(DRM_ROTATE_180) |
+ BIT(DRM_ROTATE_270));
if (desc->layout.csc) {
/*
@@ -1054,15 +1057,6 @@ atmel_hlcdc_plane_create_properties(struct drm_device *dev)
if (!props->alpha)
return ERR_PTR(-ENOMEM);
- dev->mode_config.rotation_property =
- drm_mode_create_rotation_property(dev,
- BIT(DRM_ROTATE_0) |
- BIT(DRM_ROTATE_90) |
- BIT(DRM_ROTATE_180) |
- BIT(DRM_ROTATE_270));
- if (!dev->mode_config.rotation_property)
- return ERR_PTR(-ENOMEM);
-
return props;
}
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-07-22 13:43 ` [PATCH 05/15] drm/atmel-hlcdc: " ville.syrjala
@ 2016-07-22 13:58 ` Boris Brezillon
2016-07-22 15:47 ` [PATCH v2 " ville.syrjala
1 sibling, 0 replies; 41+ messages in thread
From: Boris Brezillon @ 2016-07-22 13:58 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, dri-devel
Hi Ville,
On Fri, 22 Jul 2016 16:43:06 +0300
ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The global mode_config.rotation_property is going away, switch over to
> per-plane rotation_property.
>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index f3350c80704d..be3d4310ea97 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -903,9 +903,12 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> }
>
> if (desc->layout.xstride && desc->layout.pstride)
> - drm_object_attach_property(&plane->base.base,
> - plane->base.dev->mode_config.rotation_property,
> - BIT(DRM_ROTATE_0));
> + drm_plane_create_rotation_property(&plane->base,
> + BIT(DRM_ROTATE_0),
> + BIT(DRM_ROTATE_0) |
> + BIT(DRM_ROTATE_90) |
> + BIT(DRM_ROTATE_180) |
> + BIT(DRM_ROTATE_270));
Can you update the atmel_hlcdc_plane_init_properties() prototype to
return an int so that we can catch errors returned by
drm_plane_create_rotation_property().
Thanks,
Boris
>
> if (desc->layout.csc) {
> /*
> @@ -1054,15 +1057,6 @@ atmel_hlcdc_plane_create_properties(struct drm_device *dev)
> if (!props->alpha)
> return ERR_PTR(-ENOMEM);
>
> - dev->mode_config.rotation_property =
> - drm_mode_create_rotation_property(dev,
> - BIT(DRM_ROTATE_0) |
> - BIT(DRM_ROTATE_90) |
> - BIT(DRM_ROTATE_180) |
> - BIT(DRM_ROTATE_270));
> - if (!dev->mode_config.rotation_property)
> - return ERR_PTR(-ENOMEM);
> -
> return props;
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-07-22 13:43 ` [PATCH 05/15] drm/atmel-hlcdc: " ville.syrjala
2016-07-22 13:58 ` Boris Brezillon
@ 2016-07-22 15:47 ` ville.syrjala
2016-08-10 8:35 ` Boris Brezillon
1 sibling, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 15:47 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The global mode_config.rotation_property is going away, switch over to
per-plane rotation_property.
v2: Propagate error upwards (Boris)
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index f3350c80704d..ee9bd7a938c3 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
return 0;
}
-static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
- const struct atmel_hlcdc_layer_desc *desc,
- struct atmel_hlcdc_plane_properties *props)
+static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
+ const struct atmel_hlcdc_layer_desc *desc,
+ struct atmel_hlcdc_plane_properties *props)
{
struct regmap *regmap = plane->layer.hlcdc->regmap;
@@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
ATMEL_HLCDC_LAYER_GA_MASK);
}
- if (desc->layout.xstride && desc->layout.pstride)
- drm_object_attach_property(&plane->base.base,
- plane->base.dev->mode_config.rotation_property,
- BIT(DRM_ROTATE_0));
+ if (desc->layout.xstride && desc->layout.pstride) {
+ int ret;
+
+ ret = drm_plane_create_rotation_property(&plane->base,
+ BIT(DRM_ROTATE_0),
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_90) |
+ BIT(DRM_ROTATE_180) |
+ BIT(DRM_ROTATE_270));
+ if (ret)
+ return ret;
+ }
if (desc->layout.csc) {
/*
@@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
0x40040890);
}
+
+ return 0;
}
static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
@@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
&atmel_hlcdc_layer_plane_helper_funcs);
/* Set default property values*/
- atmel_hlcdc_plane_init_properties(plane, desc, props);
+ ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
+ if (ret)
+ return ERR_PTR(ret);
return plane;
}
@@ -1054,15 +1066,6 @@ atmel_hlcdc_plane_create_properties(struct drm_device *dev)
if (!props->alpha)
return ERR_PTR(-ENOMEM);
- dev->mode_config.rotation_property =
- drm_mode_create_rotation_property(dev,
- BIT(DRM_ROTATE_0) |
- BIT(DRM_ROTATE_90) |
- BIT(DRM_ROTATE_180) |
- BIT(DRM_ROTATE_270));
- if (!dev->mode_config.rotation_property)
- return ERR_PTR(-ENOMEM);
-
return props;
}
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-07-22 15:47 ` [PATCH v2 " ville.syrjala
@ 2016-08-10 8:35 ` Boris Brezillon
2016-08-10 9:09 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Boris Brezillon @ 2016-08-10 8:35 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, dri-devel
Hi Ville,
On Fri, 22 Jul 2016 18:47:01 +0300
ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The global mode_config.rotation_property is going away, switch over to
> per-plane rotation_property.
>
> v2: Propagate error upwards (Boris)
>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index f3350c80704d..ee9bd7a938c3 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> return 0;
> }
>
> -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> - const struct atmel_hlcdc_layer_desc *desc,
> - struct atmel_hlcdc_plane_properties *props)
> +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> + const struct atmel_hlcdc_layer_desc *desc,
> + struct atmel_hlcdc_plane_properties *props)
> {
> struct regmap *regmap = plane->layer.hlcdc->regmap;
>
> @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> ATMEL_HLCDC_LAYER_GA_MASK);
> }
>
> - if (desc->layout.xstride && desc->layout.pstride)
> - drm_object_attach_property(&plane->base.base,
> - plane->base.dev->mode_config.rotation_property,
> - BIT(DRM_ROTATE_0));
> + if (desc->layout.xstride && desc->layout.pstride) {
> + int ret;
> +
> + ret = drm_plane_create_rotation_property(&plane->base,
> + BIT(DRM_ROTATE_0),
> + BIT(DRM_ROTATE_0) |
> + BIT(DRM_ROTATE_90) |
> + BIT(DRM_ROTATE_180) |
> + BIT(DRM_ROTATE_270));
> + if (ret)
> + return ret;
> + }
>
> if (desc->layout.csc) {
> /*
> @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> 0x40040890);
> }
> +
> + return 0;
> }
>
> static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> &atmel_hlcdc_layer_plane_helper_funcs);
>
> /* Set default property values*/
> - atmel_hlcdc_plane_init_properties(plane, desc, props);
> + ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> + if (ret)
> + return ERR_PTR(ret);
Shouldn't we call drm_plane_cleanup() here?
>
> return plane;
> }
> @@ -1054,15 +1066,6 @@ atmel_hlcdc_plane_create_properties(struct drm_device *dev)
> if (!props->alpha)
> return ERR_PTR(-ENOMEM);
>
> - dev->mode_config.rotation_property =
> - drm_mode_create_rotation_property(dev,
> - BIT(DRM_ROTATE_0) |
> - BIT(DRM_ROTATE_90) |
> - BIT(DRM_ROTATE_180) |
> - BIT(DRM_ROTATE_270));
> - if (!dev->mode_config.rotation_property)
> - return ERR_PTR(-ENOMEM);
> -
> return props;
> }
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-08-10 8:35 ` Boris Brezillon
@ 2016-08-10 9:09 ` Ville Syrjälä
2016-08-10 9:25 ` Boris Brezillon
0 siblings, 1 reply; 41+ messages in thread
From: Ville Syrjälä @ 2016-08-10 9:09 UTC (permalink / raw)
To: Boris Brezillon; +Cc: intel-gfx, dri-devel
On Wed, Aug 10, 2016 at 10:35:22AM +0200, Boris Brezillon wrote:
> Hi Ville,
>
> On Fri, 22 Jul 2016 18:47:01 +0300
> ville.syrjala@linux.intel.com wrote:
>
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The global mode_config.rotation_property is going away, switch over to
> > per-plane rotation_property.
> >
> > v2: Propagate error upwards (Boris)
> >
> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> > 1 file changed, 20 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > index f3350c80704d..ee9bd7a938c3 100644
> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> > return 0;
> > }
> >
> > -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > - const struct atmel_hlcdc_layer_desc *desc,
> > - struct atmel_hlcdc_plane_properties *props)
> > +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > + const struct atmel_hlcdc_layer_desc *desc,
> > + struct atmel_hlcdc_plane_properties *props)
> > {
> > struct regmap *regmap = plane->layer.hlcdc->regmap;
> >
> > @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > ATMEL_HLCDC_LAYER_GA_MASK);
> > }
> >
> > - if (desc->layout.xstride && desc->layout.pstride)
> > - drm_object_attach_property(&plane->base.base,
> > - plane->base.dev->mode_config.rotation_property,
> > - BIT(DRM_ROTATE_0));
> > + if (desc->layout.xstride && desc->layout.pstride) {
> > + int ret;
> > +
> > + ret = drm_plane_create_rotation_property(&plane->base,
> > + BIT(DRM_ROTATE_0),
> > + BIT(DRM_ROTATE_0) |
> > + BIT(DRM_ROTATE_90) |
> > + BIT(DRM_ROTATE_180) |
> > + BIT(DRM_ROTATE_270));
> > + if (ret)
> > + return ret;
> > + }
> >
> > if (desc->layout.csc) {
> > /*
> > @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> > 0x40040890);
> > }
> > +
> > + return 0;
> > }
> >
> > static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> > @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> > &atmel_hlcdc_layer_plane_helper_funcs);
> >
> > /* Set default property values*/
> > - atmel_hlcdc_plane_init_properties(plane, desc, props);
> > + ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> > + if (ret)
> > + return ERR_PTR(ret);
>
> Shouldn't we call drm_plane_cleanup() here?
The other error path doesn't call it either, so I figured no really
cares anyway.
>
> >
> > return plane;
> > }
> > @@ -1054,15 +1066,6 @@ atmel_hlcdc_plane_create_properties(struct drm_device *dev)
> > if (!props->alpha)
> > return ERR_PTR(-ENOMEM);
> >
> > - dev->mode_config.rotation_property =
> > - drm_mode_create_rotation_property(dev,
> > - BIT(DRM_ROTATE_0) |
> > - BIT(DRM_ROTATE_90) |
> > - BIT(DRM_ROTATE_180) |
> > - BIT(DRM_ROTATE_270));
> > - if (!dev->mode_config.rotation_property)
> > - return ERR_PTR(-ENOMEM);
> > -
> > return props;
> > }
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-08-10 9:09 ` Ville Syrjälä
@ 2016-08-10 9:25 ` Boris Brezillon
2016-08-10 11:41 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Boris Brezillon @ 2016-08-10 9:25 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Wed, 10 Aug 2016 12:09:41 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Aug 10, 2016 at 10:35:22AM +0200, Boris Brezillon wrote:
> > Hi Ville,
> >
> > On Fri, 22 Jul 2016 18:47:01 +0300
> > ville.syrjala@linux.intel.com wrote:
> >
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > The global mode_config.rotation_property is going away, switch over to
> > > per-plane rotation_property.
> > >
> > > v2: Propagate error upwards (Boris)
> > >
> > > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> > > 1 file changed, 20 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > index f3350c80704d..ee9bd7a938c3 100644
> > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> > > return 0;
> > > }
> > >
> > > -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > - const struct atmel_hlcdc_layer_desc *desc,
> > > - struct atmel_hlcdc_plane_properties *props)
> > > +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > + const struct atmel_hlcdc_layer_desc *desc,
> > > + struct atmel_hlcdc_plane_properties *props)
> > > {
> > > struct regmap *regmap = plane->layer.hlcdc->regmap;
> > >
> > > @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > ATMEL_HLCDC_LAYER_GA_MASK);
> > > }
> > >
> > > - if (desc->layout.xstride && desc->layout.pstride)
> > > - drm_object_attach_property(&plane->base.base,
> > > - plane->base.dev->mode_config.rotation_property,
> > > - BIT(DRM_ROTATE_0));
> > > + if (desc->layout.xstride && desc->layout.pstride) {
> > > + int ret;
> > > +
> > > + ret = drm_plane_create_rotation_property(&plane->base,
> > > + BIT(DRM_ROTATE_0),
> > > + BIT(DRM_ROTATE_0) |
> > > + BIT(DRM_ROTATE_90) |
> > > + BIT(DRM_ROTATE_180) |
> > > + BIT(DRM_ROTATE_270));
> > > + if (ret)
> > > + return ret;
> > > + }
> > >
> > > if (desc->layout.csc) {
> > > /*
> > > @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> > > 0x40040890);
> > > }
> > > +
> > > + return 0;
> > > }
> > >
> > > static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> > > @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> > > &atmel_hlcdc_layer_plane_helper_funcs);
> > >
> > > /* Set default property values*/
> > > - atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > + ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > + if (ret)
> > > + return ERR_PTR(ret);
> >
> > Shouldn't we call drm_plane_cleanup() here?
>
> The other error path doesn't call it either, so I figured no really
> cares anyway.
Because the other error path is before plane initialization.
Now, it's true that atmel_hlcdc_create_planes() does not take care of
releasing previously registered planes in case we fail to register one
of them, but that's something we should fix instead of introducing new
potential sources of memory leaks ;).
>
> >
> > >
> > > return plane;
> > > }
> > > @@ -1054,15 +1066,6 @@ atmel_hlcdc_plane_create_properties(struct drm_device *dev)
> > > if (!props->alpha)
> > > return ERR_PTR(-ENOMEM);
> > >
> > > - dev->mode_config.rotation_property =
> > > - drm_mode_create_rotation_property(dev,
> > > - BIT(DRM_ROTATE_0) |
> > > - BIT(DRM_ROTATE_90) |
> > > - BIT(DRM_ROTATE_180) |
> > > - BIT(DRM_ROTATE_270));
> > > - if (!dev->mode_config.rotation_property)
> > > - return ERR_PTR(-ENOMEM);
> > > -
> > > return props;
> > > }
> > >
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-08-10 9:25 ` Boris Brezillon
@ 2016-08-10 11:41 ` Ville Syrjälä
2016-08-10 11:52 ` Boris Brezillon
0 siblings, 1 reply; 41+ messages in thread
From: Ville Syrjälä @ 2016-08-10 11:41 UTC (permalink / raw)
To: Boris Brezillon; +Cc: intel-gfx, dri-devel
On Wed, Aug 10, 2016 at 11:25:03AM +0200, Boris Brezillon wrote:
> On Wed, 10 Aug 2016 12:09:41 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>
> > On Wed, Aug 10, 2016 at 10:35:22AM +0200, Boris Brezillon wrote:
> > > Hi Ville,
> > >
> > > On Fri, 22 Jul 2016 18:47:01 +0300
> > > ville.syrjala@linux.intel.com wrote:
> > >
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >
> > > > The global mode_config.rotation_property is going away, switch over to
> > > > per-plane rotation_property.
> > > >
> > > > v2: Propagate error upwards (Boris)
> > > >
> > > > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> > > > 1 file changed, 20 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > index f3350c80704d..ee9bd7a938c3 100644
> > > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> > > > return 0;
> > > > }
> > > >
> > > > -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > - const struct atmel_hlcdc_layer_desc *desc,
> > > > - struct atmel_hlcdc_plane_properties *props)
> > > > +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > + const struct atmel_hlcdc_layer_desc *desc,
> > > > + struct atmel_hlcdc_plane_properties *props)
> > > > {
> > > > struct regmap *regmap = plane->layer.hlcdc->regmap;
> > > >
> > > > @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > ATMEL_HLCDC_LAYER_GA_MASK);
> > > > }
> > > >
> > > > - if (desc->layout.xstride && desc->layout.pstride)
> > > > - drm_object_attach_property(&plane->base.base,
> > > > - plane->base.dev->mode_config.rotation_property,
> > > > - BIT(DRM_ROTATE_0));
> > > > + if (desc->layout.xstride && desc->layout.pstride) {
> > > > + int ret;
> > > > +
> > > > + ret = drm_plane_create_rotation_property(&plane->base,
> > > > + BIT(DRM_ROTATE_0),
> > > > + BIT(DRM_ROTATE_0) |
> > > > + BIT(DRM_ROTATE_90) |
> > > > + BIT(DRM_ROTATE_180) |
> > > > + BIT(DRM_ROTATE_270));
> > > > + if (ret)
> > > > + return ret;
> > > > + }
> > > >
> > > > if (desc->layout.csc) {
> > > > /*
> > > > @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> > > > 0x40040890);
> > > > }
> > > > +
> > > > + return 0;
> > > > }
> > > >
> > > > static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> > > > @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> > > > &atmel_hlcdc_layer_plane_helper_funcs);
> > > >
> > > > /* Set default property values*/
> > > > - atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > + ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > + if (ret)
> > > > + return ERR_PTR(ret);
> > >
> > > Shouldn't we call drm_plane_cleanup() here?
> >
> > The other error path doesn't call it either, so I figured no really
> > cares anyway.
>
> Because the other error path is before plane initialization.
There's some stuff being done earlier already, so either that part
doesn't need cleaning, or the init/cleanup is somehow asymmetric, or
it's just broken. Anyways, as my interest in cleaning up this driver
is as near zero as can be measured, I think I'll leave all that to
someone else to figure out.
> Now, it's true that atmel_hlcdc_create_planes() does not take care of
> releasing previously registered planes in case we fail to register one
> of them, but that's something we should fix instead of introducing new
> potential sources of memory leaks ;).
>
> >
> > >
> > > >
> > > > return plane;
> > > > }
> > > > @@ -1054,15 +1066,6 @@ atmel_hlcdc_plane_create_properties(struct drm_device *dev)
> > > > if (!props->alpha)
> > > > return ERR_PTR(-ENOMEM);
> > > >
> > > > - dev->mode_config.rotation_property =
> > > > - drm_mode_create_rotation_property(dev,
> > > > - BIT(DRM_ROTATE_0) |
> > > > - BIT(DRM_ROTATE_90) |
> > > > - BIT(DRM_ROTATE_180) |
> > > > - BIT(DRM_ROTATE_270));
> > > > - if (!dev->mode_config.rotation_property)
> > > > - return ERR_PTR(-ENOMEM);
> > > > -
> > > > return props;
> > > > }
> > > >
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-08-10 11:41 ` Ville Syrjälä
@ 2016-08-10 11:52 ` Boris Brezillon
2016-08-10 12:04 ` Boris Brezillon
2016-08-10 14:04 ` Daniel Vetter
0 siblings, 2 replies; 41+ messages in thread
From: Boris Brezillon @ 2016-08-10 11:52 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Wed, 10 Aug 2016 14:41:52 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Aug 10, 2016 at 11:25:03AM +0200, Boris Brezillon wrote:
> > On Wed, 10 Aug 2016 12:09:41 +0300
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >
> > > On Wed, Aug 10, 2016 at 10:35:22AM +0200, Boris Brezillon wrote:
> > > > Hi Ville,
> > > >
> > > > On Fri, 22 Jul 2016 18:47:01 +0300
> > > > ville.syrjala@linux.intel.com wrote:
> > > >
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > >
> > > > > The global mode_config.rotation_property is going away, switch over to
> > > > > per-plane rotation_property.
> > > > >
> > > > > v2: Propagate error upwards (Boris)
> > > > >
> > > > > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> > > > > 1 file changed, 20 insertions(+), 17 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > index f3350c80704d..ee9bd7a938c3 100644
> > > > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > - const struct atmel_hlcdc_layer_desc *desc,
> > > > > - struct atmel_hlcdc_plane_properties *props)
> > > > > +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > + const struct atmel_hlcdc_layer_desc *desc,
> > > > > + struct atmel_hlcdc_plane_properties *props)
> > > > > {
> > > > > struct regmap *regmap = plane->layer.hlcdc->regmap;
> > > > >
> > > > > @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > ATMEL_HLCDC_LAYER_GA_MASK);
> > > > > }
> > > > >
> > > > > - if (desc->layout.xstride && desc->layout.pstride)
> > > > > - drm_object_attach_property(&plane->base.base,
> > > > > - plane->base.dev->mode_config.rotation_property,
> > > > > - BIT(DRM_ROTATE_0));
> > > > > + if (desc->layout.xstride && desc->layout.pstride) {
> > > > > + int ret;
> > > > > +
> > > > > + ret = drm_plane_create_rotation_property(&plane->base,
> > > > > + BIT(DRM_ROTATE_0),
> > > > > + BIT(DRM_ROTATE_0) |
> > > > > + BIT(DRM_ROTATE_90) |
> > > > > + BIT(DRM_ROTATE_180) |
> > > > > + BIT(DRM_ROTATE_270));
> > > > > + if (ret)
> > > > > + return ret;
> > > > > + }
> > > > >
> > > > > if (desc->layout.csc) {
> > > > > /*
> > > > > @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> > > > > 0x40040890);
> > > > > }
> > > > > +
> > > > > + return 0;
> > > > > }
> > > > >
> > > > > static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> > > > > @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> > > > > &atmel_hlcdc_layer_plane_helper_funcs);
> > > > >
> > > > > /* Set default property values*/
> > > > > - atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > + ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > + if (ret)
> > > > > + return ERR_PTR(ret);
> > > >
> > > > Shouldn't we call drm_plane_cleanup() here?
> > >
> > > The other error path doesn't call it either, so I figured no really
> > > cares anyway.
> >
> > Because the other error path is before plane initialization.
>
> There's some stuff being done earlier already, so either that part
> doesn't need cleaning, or the init/cleanup is somehow asymmetric, or
> it's just broken.
devm_ resources are automatically released by the device-model, hence
the absence of devm_kfree(). atmel_hlcdc_layer_cleanup() should be
called if drm_universal_plane_init() fails though.
> Anyways, as my interest in cleaning up this driver
> is as near zero as can be measured, I think I'll leave all that to
> someone else to figure out.
I'm not asking you to fix existing problems, but you're introducing a
potential new source of leak here.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-08-10 11:52 ` Boris Brezillon
@ 2016-08-10 12:04 ` Boris Brezillon
2016-08-10 14:04 ` Daniel Vetter
1 sibling, 0 replies; 41+ messages in thread
From: Boris Brezillon @ 2016-08-10 12:04 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Wed, 10 Aug 2016 13:52:45 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> On Wed, 10 Aug 2016 14:41:52 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>
> > On Wed, Aug 10, 2016 at 11:25:03AM +0200, Boris Brezillon wrote:
> > > On Wed, 10 Aug 2016 12:09:41 +0300
> > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > >
> > > > On Wed, Aug 10, 2016 at 10:35:22AM +0200, Boris Brezillon wrote:
> > > > > Hi Ville,
> > > > >
> > > > > On Fri, 22 Jul 2016 18:47:01 +0300
> > > > > ville.syrjala@linux.intel.com wrote:
> > > > >
> > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > >
> > > > > > The global mode_config.rotation_property is going away, switch over to
> > > > > > per-plane rotation_property.
> > > > > >
> > > > > > v2: Propagate error upwards (Boris)
> > > > > >
> > > > > > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> > > > > > 1 file changed, 20 insertions(+), 17 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > index f3350c80704d..ee9bd7a938c3 100644
> > > > > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > - const struct atmel_hlcdc_layer_desc *desc,
> > > > > > - struct atmel_hlcdc_plane_properties *props)
> > > > > > +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > + const struct atmel_hlcdc_layer_desc *desc,
> > > > > > + struct atmel_hlcdc_plane_properties *props)
> > > > > > {
> > > > > > struct regmap *regmap = plane->layer.hlcdc->regmap;
> > > > > >
> > > > > > @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > ATMEL_HLCDC_LAYER_GA_MASK);
> > > > > > }
> > > > > >
> > > > > > - if (desc->layout.xstride && desc->layout.pstride)
> > > > > > - drm_object_attach_property(&plane->base.base,
> > > > > > - plane->base.dev->mode_config.rotation_property,
> > > > > > - BIT(DRM_ROTATE_0));
> > > > > > + if (desc->layout.xstride && desc->layout.pstride) {
> > > > > > + int ret;
> > > > > > +
> > > > > > + ret = drm_plane_create_rotation_property(&plane->base,
> > > > > > + BIT(DRM_ROTATE_0),
> > > > > > + BIT(DRM_ROTATE_0) |
> > > > > > + BIT(DRM_ROTATE_90) |
> > > > > > + BIT(DRM_ROTATE_180) |
> > > > > > + BIT(DRM_ROTATE_270));
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > + }
> > > > > >
> > > > > > if (desc->layout.csc) {
> > > > > > /*
> > > > > > @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> > > > > > 0x40040890);
> > > > > > }
> > > > > > +
> > > > > > + return 0;
> > > > > > }
> > > > > >
> > > > > > static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> > > > > > @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> > > > > > &atmel_hlcdc_layer_plane_helper_funcs);
> > > > > >
> > > > > > /* Set default property values*/
> > > > > > - atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > > + ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > > + if (ret)
> > > > > > + return ERR_PTR(ret);
> > > > >
> > > > > Shouldn't we call drm_plane_cleanup() here?
> > > >
> > > > The other error path doesn't call it either, so I figured no really
> > > > cares anyway.
> > >
> > > Because the other error path is before plane initialization.
> >
> > There's some stuff being done earlier already, so either that part
> > doesn't need cleaning, or the init/cleanup is somehow asymmetric, or
> > it's just broken.
>
> devm_ resources are automatically released by the device-model, hence
> the absence of devm_kfree(). atmel_hlcdc_layer_cleanup() should be
> called if drm_universal_plane_init() fails though.
>
> > Anyways, as my interest in cleaning up this driver
> > is as near zero as can be measured, I think I'll leave all that to
> > someone else to figure out.
>
> I'm not asking you to fix existing problems, but you're introducing a
> potential new source of leak here.
Nevermind, I'll fix that myself. Here is the ack you were waiting for.
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-08-10 11:52 ` Boris Brezillon
2016-08-10 12:04 ` Boris Brezillon
@ 2016-08-10 14:04 ` Daniel Vetter
2016-08-10 16:38 ` Boris Brezillon
1 sibling, 1 reply; 41+ messages in thread
From: Daniel Vetter @ 2016-08-10 14:04 UTC (permalink / raw)
To: Boris Brezillon; +Cc: intel-gfx, dri-devel
On Wed, Aug 10, 2016 at 01:52:45PM +0200, Boris Brezillon wrote:
> On Wed, 10 Aug 2016 14:41:52 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>
> > On Wed, Aug 10, 2016 at 11:25:03AM +0200, Boris Brezillon wrote:
> > > On Wed, 10 Aug 2016 12:09:41 +0300
> > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > >
> > > > On Wed, Aug 10, 2016 at 10:35:22AM +0200, Boris Brezillon wrote:
> > > > > Hi Ville,
> > > > >
> > > > > On Fri, 22 Jul 2016 18:47:01 +0300
> > > > > ville.syrjala@linux.intel.com wrote:
> > > > >
> > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > >
> > > > > > The global mode_config.rotation_property is going away, switch over to
> > > > > > per-plane rotation_property.
> > > > > >
> > > > > > v2: Propagate error upwards (Boris)
> > > > > >
> > > > > > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> > > > > > 1 file changed, 20 insertions(+), 17 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > index f3350c80704d..ee9bd7a938c3 100644
> > > > > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > - const struct atmel_hlcdc_layer_desc *desc,
> > > > > > - struct atmel_hlcdc_plane_properties *props)
> > > > > > +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > + const struct atmel_hlcdc_layer_desc *desc,
> > > > > > + struct atmel_hlcdc_plane_properties *props)
> > > > > > {
> > > > > > struct regmap *regmap = plane->layer.hlcdc->regmap;
> > > > > >
> > > > > > @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > ATMEL_HLCDC_LAYER_GA_MASK);
> > > > > > }
> > > > > >
> > > > > > - if (desc->layout.xstride && desc->layout.pstride)
> > > > > > - drm_object_attach_property(&plane->base.base,
> > > > > > - plane->base.dev->mode_config.rotation_property,
> > > > > > - BIT(DRM_ROTATE_0));
> > > > > > + if (desc->layout.xstride && desc->layout.pstride) {
> > > > > > + int ret;
> > > > > > +
> > > > > > + ret = drm_plane_create_rotation_property(&plane->base,
> > > > > > + BIT(DRM_ROTATE_0),
> > > > > > + BIT(DRM_ROTATE_0) |
> > > > > > + BIT(DRM_ROTATE_90) |
> > > > > > + BIT(DRM_ROTATE_180) |
> > > > > > + BIT(DRM_ROTATE_270));
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > + }
> > > > > >
> > > > > > if (desc->layout.csc) {
> > > > > > /*
> > > > > > @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> > > > > > 0x40040890);
> > > > > > }
> > > > > > +
> > > > > > + return 0;
> > > > > > }
> > > > > >
> > > > > > static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> > > > > > @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> > > > > > &atmel_hlcdc_layer_plane_helper_funcs);
> > > > > >
> > > > > > /* Set default property values*/
> > > > > > - atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > > + ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > > + if (ret)
> > > > > > + return ERR_PTR(ret);
> > > > >
> > > > > Shouldn't we call drm_plane_cleanup() here?
> > > >
> > > > The other error path doesn't call it either, so I figured no really
> > > > cares anyway.
> > >
> > > Because the other error path is before plane initialization.
> >
> > There's some stuff being done earlier already, so either that part
> > doesn't need cleaning, or the init/cleanup is somehow asymmetric, or
> > it's just broken.
>
> devm_ resources are automatically released by the device-model, hence
> the absence of devm_kfree(). atmel_hlcdc_layer_cleanup() should be
> called if drm_universal_plane_init() fails though.
Aside: drm objects are independently refcounting, mostly likely using
devm_ on the underlying physical device for drm objects is a bug. At least
all this stuff looks extremely fishy.
-Daniel
>
> > Anyways, as my interest in cleaning up this driver
> > is as near zero as can be measured, I think I'll leave all that to
> > someone else to figure out.
>
> I'm not asking you to fix existing problems, but you're introducing a
> potential new source of leak here.
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-08-10 14:04 ` Daniel Vetter
@ 2016-08-10 16:38 ` Boris Brezillon
2016-08-11 8:50 ` Daniel Vetter
0 siblings, 1 reply; 41+ messages in thread
From: Boris Brezillon @ 2016-08-10 16:38 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
On Wed, 10 Aug 2016 16:04:37 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Aug 10, 2016 at 01:52:45PM +0200, Boris Brezillon wrote:
> > On Wed, 10 Aug 2016 14:41:52 +0300
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >
> > > On Wed, Aug 10, 2016 at 11:25:03AM +0200, Boris Brezillon wrote:
> > > > On Wed, 10 Aug 2016 12:09:41 +0300
> > > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > >
> > > > > On Wed, Aug 10, 2016 at 10:35:22AM +0200, Boris Brezillon wrote:
> > > > > > Hi Ville,
> > > > > >
> > > > > > On Fri, 22 Jul 2016 18:47:01 +0300
> > > > > > ville.syrjala@linux.intel.com wrote:
> > > > > >
> > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > >
> > > > > > > The global mode_config.rotation_property is going away, switch over to
> > > > > > > per-plane rotation_property.
> > > > > > >
> > > > > > > v2: Propagate error upwards (Boris)
> > > > > > >
> > > > > > > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > ---
> > > > > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> > > > > > > 1 file changed, 20 insertions(+), 17 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > > index f3350c80704d..ee9bd7a938c3 100644
> > > > > > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > > @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> > > > > > > return 0;
> > > > > > > }
> > > > > > >
> > > > > > > -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > > - const struct atmel_hlcdc_layer_desc *desc,
> > > > > > > - struct atmel_hlcdc_plane_properties *props)
> > > > > > > +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > > + const struct atmel_hlcdc_layer_desc *desc,
> > > > > > > + struct atmel_hlcdc_plane_properties *props)
> > > > > > > {
> > > > > > > struct regmap *regmap = plane->layer.hlcdc->regmap;
> > > > > > >
> > > > > > > @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > > ATMEL_HLCDC_LAYER_GA_MASK);
> > > > > > > }
> > > > > > >
> > > > > > > - if (desc->layout.xstride && desc->layout.pstride)
> > > > > > > - drm_object_attach_property(&plane->base.base,
> > > > > > > - plane->base.dev->mode_config.rotation_property,
> > > > > > > - BIT(DRM_ROTATE_0));
> > > > > > > + if (desc->layout.xstride && desc->layout.pstride) {
> > > > > > > + int ret;
> > > > > > > +
> > > > > > > + ret = drm_plane_create_rotation_property(&plane->base,
> > > > > > > + BIT(DRM_ROTATE_0),
> > > > > > > + BIT(DRM_ROTATE_0) |
> > > > > > > + BIT(DRM_ROTATE_90) |
> > > > > > > + BIT(DRM_ROTATE_180) |
> > > > > > > + BIT(DRM_ROTATE_270));
> > > > > > > + if (ret)
> > > > > > > + return ret;
> > > > > > > + }
> > > > > > >
> > > > > > > if (desc->layout.csc) {
> > > > > > > /*
> > > > > > > @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > > ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> > > > > > > 0x40040890);
> > > > > > > }
> > > > > > > +
> > > > > > > + return 0;
> > > > > > > }
> > > > > > >
> > > > > > > static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> > > > > > > @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> > > > > > > &atmel_hlcdc_layer_plane_helper_funcs);
> > > > > > >
> > > > > > > /* Set default property values*/
> > > > > > > - atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > > > + ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > > > + if (ret)
> > > > > > > + return ERR_PTR(ret);
> > > > > >
> > > > > > Shouldn't we call drm_plane_cleanup() here?
> > > > >
> > > > > The other error path doesn't call it either, so I figured no really
> > > > > cares anyway.
> > > >
> > > > Because the other error path is before plane initialization.
> > >
> > > There's some stuff being done earlier already, so either that part
> > > doesn't need cleaning, or the init/cleanup is somehow asymmetric, or
> > > it's just broken.
> >
> > devm_ resources are automatically released by the device-model, hence
> > the absence of devm_kfree(). atmel_hlcdc_layer_cleanup() should be
> > called if drm_universal_plane_init() fails though.
>
> Aside: drm objects are independently refcounting, mostly likely using
> devm_ on the underlying physical device for drm objects is a bug.
AFAICT it's not buggy: I'm not freeing the plane object in ->destroy()
(this is done when the underlying device is destroyed), but I release
other resources.
> At least all this stuff looks extremely fishy.
But that's true, using devm_ here is probably not the best solution.
I'll try to change that along with the memory leak fixes.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property
2016-08-10 16:38 ` Boris Brezillon
@ 2016-08-11 8:50 ` Daniel Vetter
0 siblings, 0 replies; 41+ messages in thread
From: Daniel Vetter @ 2016-08-11 8:50 UTC (permalink / raw)
To: Boris Brezillon; +Cc: dri-devel, intel-gfx
On Wed, Aug 10, 2016 at 06:38:39PM +0200, Boris Brezillon wrote:
> On Wed, 10 Aug 2016 16:04:37 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > On Wed, Aug 10, 2016 at 01:52:45PM +0200, Boris Brezillon wrote:
> > > On Wed, 10 Aug 2016 14:41:52 +0300
> > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > >
> > > > On Wed, Aug 10, 2016 at 11:25:03AM +0200, Boris Brezillon wrote:
> > > > > On Wed, 10 Aug 2016 12:09:41 +0300
> > > > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > > >
> > > > > > On Wed, Aug 10, 2016 at 10:35:22AM +0200, Boris Brezillon wrote:
> > > > > > > Hi Ville,
> > > > > > >
> > > > > > > On Fri, 22 Jul 2016 18:47:01 +0300
> > > > > > > ville.syrjala@linux.intel.com wrote:
> > > > > > >
> > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > >
> > > > > > > > The global mode_config.rotation_property is going away, switch over to
> > > > > > > > per-plane rotation_property.
> > > > > > > >
> > > > > > > > v2: Propagate error upwards (Boris)
> > > > > > > >
> > > > > > > > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > ---
> > > > > > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> > > > > > > > 1 file changed, 20 insertions(+), 17 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > > > index f3350c80704d..ee9bd7a938c3 100644
> > > > > > > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > > > > > > @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> > > > > > > > return 0;
> > > > > > > > }
> > > > > > > >
> > > > > > > > -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > > > - const struct atmel_hlcdc_layer_desc *desc,
> > > > > > > > - struct atmel_hlcdc_plane_properties *props)
> > > > > > > > +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > > > + const struct atmel_hlcdc_layer_desc *desc,
> > > > > > > > + struct atmel_hlcdc_plane_properties *props)
> > > > > > > > {
> > > > > > > > struct regmap *regmap = plane->layer.hlcdc->regmap;
> > > > > > > >
> > > > > > > > @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > > > ATMEL_HLCDC_LAYER_GA_MASK);
> > > > > > > > }
> > > > > > > >
> > > > > > > > - if (desc->layout.xstride && desc->layout.pstride)
> > > > > > > > - drm_object_attach_property(&plane->base.base,
> > > > > > > > - plane->base.dev->mode_config.rotation_property,
> > > > > > > > - BIT(DRM_ROTATE_0));
> > > > > > > > + if (desc->layout.xstride && desc->layout.pstride) {
> > > > > > > > + int ret;
> > > > > > > > +
> > > > > > > > + ret = drm_plane_create_rotation_property(&plane->base,
> > > > > > > > + BIT(DRM_ROTATE_0),
> > > > > > > > + BIT(DRM_ROTATE_0) |
> > > > > > > > + BIT(DRM_ROTATE_90) |
> > > > > > > > + BIT(DRM_ROTATE_180) |
> > > > > > > > + BIT(DRM_ROTATE_270));
> > > > > > > > + if (ret)
> > > > > > > > + return ret;
> > > > > > > > + }
> > > > > > > >
> > > > > > > > if (desc->layout.csc) {
> > > > > > > > /*
> > > > > > > > @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > > > > > > ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> > > > > > > > 0x40040890);
> > > > > > > > }
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > }
> > > > > > > >
> > > > > > > > static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> > > > > > > > @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> > > > > > > > &atmel_hlcdc_layer_plane_helper_funcs);
> > > > > > > >
> > > > > > > > /* Set default property values*/
> > > > > > > > - atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > > > > + ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > > > > > > + if (ret)
> > > > > > > > + return ERR_PTR(ret);
> > > > > > >
> > > > > > > Shouldn't we call drm_plane_cleanup() here?
> > > > > >
> > > > > > The other error path doesn't call it either, so I figured no really
> > > > > > cares anyway.
> > > > >
> > > > > Because the other error path is before plane initialization.
> > > >
> > > > There's some stuff being done earlier already, so either that part
> > > > doesn't need cleaning, or the init/cleanup is somehow asymmetric, or
> > > > it's just broken.
> > >
> > > devm_ resources are automatically released by the device-model, hence
> > > the absence of devm_kfree(). atmel_hlcdc_layer_cleanup() should be
> > > called if drm_universal_plane_init() fails though.
> >
> > Aside: drm objects are independently refcounting, mostly likely using
> > devm_ on the underlying physical device for drm objects is a bug.
>
> AFAICT it's not buggy: I'm not freeing the plane object in ->destroy()
> (this is done when the underlying device is destroyed), but I release
> other resources.
Well, drm_device is also refcounted, so you can't allocate anything in
there using devm_ on some other hw device. At least not if you really want
hotplug to work. I guess we could switch the refcounting for drm_device to
struct device (instead of our own) and then you could use the devm_
functions on that virtual object. But since no one yet figured out how to
exactly do drm_device hot-unplugging it's all a bit unclear. Unfortunately
udl doesn't support current generation of usb display chips since years,
so there's not really much of a user left :(
Anyway all rather academic with soc gpus, and for driver module unload the
two actions happen in the right order so it's all fine.
-Daniel
>
> > At least all this stuff looks extremely fishy.
>
> But that's true, using devm_ here is probably not the best solution.
>
> I'll try to change that along with the memory leak fixes.
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 06/15] drm/omap: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (4 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 05/15] drm/atmel-hlcdc: " ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 13:43 ` [PATCH 07/15] drm/omap: Use per-plane rotation property ville.syrjala
` (10 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Tomi Valkeinen
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
0 isn't a valid rotation property value, so let's set the initial value
of the property to BIT(DRM_ROTATE_0) instead.
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/omapdrm/omap_drv.c | 6 ++++--
drivers/gpu/drm/omapdrm/omap_plane.c | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 26c6134eb744..f57a6910ac01 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -761,12 +761,14 @@ static void dev_lastclose(struct drm_device *dev)
*/
for (i = 0; i < priv->num_crtcs; i++) {
drm_object_property_set_value(&priv->crtcs[i]->base,
- dev->mode_config.rotation_property, 0);
+ dev->mode_config.rotation_property,
+ BIT(DRM_ROTATE_0));
}
for (i = 0; i < priv->num_planes; i++) {
drm_object_property_set_value(&priv->planes[i]->base,
- dev->mode_config.rotation_property, 0);
+ dev->mode_config.rotation_property,
+ BIT(DRM_ROTATE_0));
}
}
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 7f0d567b8d67..08e911469a2f 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -213,7 +213,7 @@ void omap_plane_install_properties(struct drm_plane *plane,
if (priv->has_dmm) {
struct drm_property *prop = dev->mode_config.rotation_property;
- drm_object_attach_property(obj, prop, 0);
+ drm_object_attach_property(obj, prop, BIT(DRM_ROTATE_0));
}
drm_object_attach_property(obj, priv->zorder_prop, 0);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 07/15] drm/omap: Use per-plane rotation property
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (5 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 06/15] drm/omap: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0 ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-08-11 11:32 ` Tomi Valkeinen
2016-07-22 13:43 ` [PATCH 08/15] drm/msm/mdp5: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0 ville.syrjala
` (9 subsequent siblings)
16 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Tomi Valkeinen
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The global mode_config.rotation_property is going away, switch over to
per-plane rotation_property.
Not sure I got the annoying crtc rotation_property handling right.
Might work, or migth not.
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/omapdrm/omap_crtc.c | 13 ++++-----
drivers/gpu/drm/omapdrm/omap_drv.c | 52 +++++++++++++++++-------------------
drivers/gpu/drm/omapdrm/omap_plane.c | 12 ++++++---
3 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 180f644e861e..16c691dbc372 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -438,13 +438,14 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
}
}
-static bool omap_crtc_is_plane_prop(struct drm_device *dev,
+static bool omap_crtc_is_plane_prop(struct drm_crtc *crtc,
struct drm_property *property)
{
+ struct drm_device *dev = crtc->dev;
struct omap_drm_private *priv = dev->dev_private;
return property == priv->zorder_prop ||
- property == dev->mode_config.rotation_property;
+ property == crtc->primary->rotation_property;
}
static int omap_crtc_atomic_set_property(struct drm_crtc *crtc,
@@ -452,9 +453,7 @@ static int omap_crtc_atomic_set_property(struct drm_crtc *crtc,
struct drm_property *property,
uint64_t val)
{
- struct drm_device *dev = crtc->dev;
-
- if (omap_crtc_is_plane_prop(dev, property)) {
+ if (omap_crtc_is_plane_prop(crtc, property)) {
struct drm_plane_state *plane_state;
struct drm_plane *plane = crtc->primary;
@@ -479,9 +478,7 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
struct drm_property *property,
uint64_t *val)
{
- struct drm_device *dev = crtc->dev;
-
- if (omap_crtc_is_plane_prop(dev, property)) {
+ if (omap_crtc_is_plane_prop(crtc, property)) {
/*
* Delegate property get to the primary plane. The
* drm_atomic_plane_get_property() function isn't exported, but
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index f57a6910ac01..98040ec836f1 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -292,16 +292,6 @@ static int omap_modeset_init_properties(struct drm_device *dev)
{
struct omap_drm_private *priv = dev->dev_private;
- if (priv->has_dmm) {
- dev->mode_config.rotation_property =
- drm_mode_create_rotation_property(dev,
- BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
- BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
- BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
- if (!dev->mode_config.rotation_property)
- return -ENOMEM;
- }
-
priv->zorder_prop = drm_property_create_range(dev, 0, "zorder", 0, 3);
if (!priv->zorder_prop)
return -ENOMEM;
@@ -752,24 +742,32 @@ static void dev_lastclose(struct drm_device *dev)
DBG("lastclose: dev=%p", dev);
- if (dev->mode_config.rotation_property) {
- /* need to restore default rotation state.. not sure
- * if there is a cleaner way to restore properties to
- * default state? Maybe a flag that properties should
- * automatically be restored to default state on
- * lastclose?
- */
- for (i = 0; i < priv->num_crtcs; i++) {
- drm_object_property_set_value(&priv->crtcs[i]->base,
- dev->mode_config.rotation_property,
- BIT(DRM_ROTATE_0));
- }
+ /* need to restore default rotation state.. not sure
+ * if there is a cleaner way to restore properties to
+ * default state? Maybe a flag that properties should
+ * automatically be restored to default state on
+ * lastclose?
+ */
+ for (i = 0; i < priv->num_crtcs; i++) {
+ struct drm_crtc *crtc = priv->crtcs[i];
- for (i = 0; i < priv->num_planes; i++) {
- drm_object_property_set_value(&priv->planes[i]->base,
- dev->mode_config.rotation_property,
- BIT(DRM_ROTATE_0));
- }
+ if (!crtc->primary->rotation_property)
+ continue;
+
+ drm_object_property_set_value(&crtc->base,
+ crtc->primary->rotation_property,
+ BIT(DRM_ROTATE_0));
+ }
+
+ for (i = 0; i < priv->num_planes; i++) {
+ struct drm_plane *plane = priv->planes[i];
+
+ if (!plane->rotation_property)
+ continue;
+
+ drm_object_property_set_value(&plane->base,
+ plane->rotation_property,
+ BIT(DRM_ROTATE_0));
}
if (priv->fbdev) {
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 08e911469a2f..c1df4c0422cb 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -211,9 +211,15 @@ void omap_plane_install_properties(struct drm_plane *plane,
struct omap_drm_private *priv = dev->dev_private;
if (priv->has_dmm) {
- struct drm_property *prop = dev->mode_config.rotation_property;
-
- drm_object_attach_property(obj, prop, BIT(DRM_ROTATE_0));
+ drm_plane_create_rotation_property(plane,
+ BIT(DRM_ROTATE_0),
+ BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
+ BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
+ BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
+
+ if (plane->rotation_property && obj != &plane->base)
+ drm_object_attach_property(obj, plane->rotation_property,
+ BIT(DRM_ROTATE_0));
}
drm_object_attach_property(obj, priv->zorder_prop, 0);
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 07/15] drm/omap: Use per-plane rotation property
2016-07-22 13:43 ` [PATCH 07/15] drm/omap: Use per-plane rotation property ville.syrjala
@ 2016-08-11 11:32 ` Tomi Valkeinen
2016-08-11 13:33 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Tomi Valkeinen @ 2016-08-11 11:32 UTC (permalink / raw)
To: ville.syrjala, dri-devel; +Cc: intel-gfx, Tomi Valkeinen
[-- Attachment #1.1.1: Type: text/plain, Size: 1117 bytes --]
Hi,
On 22/07/16 16:43, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The global mode_config.rotation_property is going away, switch over to
> per-plane rotation_property.
>
> Not sure I got the annoying crtc rotation_property handling right.
> Might work, or migth not.
I think something is funny with this patch or the series. I fetched your
branch, and with your series, it looks like the primary planes lose all
their props. modetest says:
could not get plane 26 properties: Invalid argument
could not get plane 30 properties: Invalid argument
and
Planes:
id crtc fb CRTC x,y x,y gamma size possible
crtcs
26 28 55 0,0 0,0 0 0x00000001
formats: RG16 RX12 XR12 RA12 AR12 XR15 AR15 RG24 RX24 XR24 RA24 AR24
no properties found
30 0 0 0,0 0,0 0 0x00000002
formats: RG16 RX12 XR12 RA12 AR12 XR15 AR15 RG24 RX24 XR24 RA24 AR24
NV12 YUYV UYVY
no properties found
I didn't look closer yet.
Tomi
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 07/15] drm/omap: Use per-plane rotation property
2016-08-11 11:32 ` Tomi Valkeinen
@ 2016-08-11 13:33 ` Ville Syrjälä
2016-08-12 16:04 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Ville Syrjälä @ 2016-08-11 13:33 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: intel-gfx, Tomi Valkeinen, dri-devel
On Thu, Aug 11, 2016 at 02:32:44PM +0300, Tomi Valkeinen wrote:
> Hi,
>
> On 22/07/16 16:43, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The global mode_config.rotation_property is going away, switch over to
> > per-plane rotation_property.
> >
> > Not sure I got the annoying crtc rotation_property handling right.
> > Might work, or migth not.
>
> I think something is funny with this patch or the series. I fetched your
> branch, and with your series, it looks like the primary planes lose all
> their props. modetest says:
>
> could not get plane 26 properties: Invalid argument
> could not get plane 30 properties: Invalid argument
Hmm. Weird. Is it really the get props ioctl that fails?
The first EINVAL I can spot there is
if (!obj->properties) {
ret = -EINVAL;
goto out_unref;
}
which definitely makes no sense since this is assigned
as plane->base.properties = &plane->properties. So can't be that unless
we manage to clear the pointer somehow after the init.
The only other direct EINVAL I see there is if
drm_object_property_get_value(obj->properties->properties[i])
fails to find the passed prop in the properties array. Which clearly
can't happen since we got it from the array in the first place. Also,
clearly that code is rather inefficient, perhaps someone should rewrite
it a bit.
Can't quite see how this could fail for the plane in other ways. But I
might be blind.
>
> and
>
> Planes:
> id crtc fb CRTC x,y x,y gamma size possible
> crtcs
> 26 28 55 0,0 0,0 0 0x00000001
> formats: RG16 RX12 XR12 RA12 AR12 XR15 AR15 RG24 RX24 XR24 RA24 AR24
> no properties found
> 30 0 0 0,0 0,0 0 0x00000002
> formats: RG16 RX12 XR12 RA12 AR12 XR15 AR15 RG24 RX24 XR24 RA24 AR24
> NV12 YUYV UYVY
> no properties found
>
> I didn't look closer yet.
>
> Tomi
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 07/15] drm/omap: Use per-plane rotation property
2016-08-11 13:33 ` Ville Syrjälä
@ 2016-08-12 16:04 ` Ville Syrjälä
2016-09-23 11:33 ` [Intel-gfx] " Tomi Valkeinen
0 siblings, 1 reply; 41+ messages in thread
From: Ville Syrjälä @ 2016-08-12 16:04 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: intel-gfx, Tomi Valkeinen, dri-devel
On Thu, Aug 11, 2016 at 04:33:32PM +0300, Ville Syrjälä wrote:
> On Thu, Aug 11, 2016 at 02:32:44PM +0300, Tomi Valkeinen wrote:
> > Hi,
> >
> > On 22/07/16 16:43, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > The global mode_config.rotation_property is going away, switch over to
> > > per-plane rotation_property.
> > >
> > > Not sure I got the annoying crtc rotation_property handling right.
> > > Might work, or migth not.
> >
> > I think something is funny with this patch or the series. I fetched your
> > branch, and with your series, it looks like the primary planes lose all
> > their props. modetest says:
> >
> > could not get plane 26 properties: Invalid argument
> > could not get plane 30 properties: Invalid argument
>
> Hmm. Weird. Is it really the get props ioctl that fails?
>
> The first EINVAL I can spot there is
> if (!obj->properties) {
> ret = -EINVAL;
> goto out_unref;
> }
> which definitely makes no sense since this is assigned
> as plane->base.properties = &plane->properties. So can't be that unless
> we manage to clear the pointer somehow after the init.
>
> The only other direct EINVAL I see there is if
> drm_object_property_get_value(obj->properties->properties[i])
> fails to find the passed prop in the properties array. Which clearly
> can't happen since we got it from the array in the first place. Also,
> clearly that code is rather inefficient, perhaps someone should rewrite
> it a bit.
>
> Can't quite see how this could fail for the plane in other ways. But I
> might be blind.
I tried to think on this a bit more, and the only think I came up with was
that we end up doing the drm_plane_create_rotation_property() twice for the
primary planes. I tried that on i915 but it'd didn't result in anything bad
AFAICS. Would leak a bit, but so what :P
Dunno, I guess you could try something like:
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -211,11 +211,12 @@ void omap_plane_install_properties(struct drm_plane *plane,
struct omap_drm_private *priv = dev->dev_private;
if (priv->has_dmm) {
- drm_plane_create_rotation_property(plane,
- BIT(DRM_ROTATE_0),
- BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
- BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
- BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
+ if (!plane->rotation_property)
+ drm_plane_create_rotation_property(plane,
+ BIT(DRM_ROTATE_0),
+ BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
+ BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
+ BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
>
> >
> > and
> >
> > Planes:
> > id crtc fb CRTC x,y x,y gamma size possible
> > crtcs
> > 26 28 55 0,0 0,0 0 0x00000001
> > formats: RG16 RX12 XR12 RA12 AR12 XR15 AR15 RG24 RX24 XR24 RA24 AR24
> > no properties found
> > 30 0 0 0,0 0,0 0 0x00000002
> > formats: RG16 RX12 XR12 RA12 AR12 XR15 AR15 RG24 RX24 XR24 RA24 AR24
> > NV12 YUYV UYVY
> > no properties found
> >
> > I didn't look closer yet.
> >
> > Tomi
> >
>
>
>
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [Intel-gfx] [PATCH 07/15] drm/omap: Use per-plane rotation property
2016-08-12 16:04 ` Ville Syrjälä
@ 2016-09-23 11:33 ` Tomi Valkeinen
0 siblings, 0 replies; 41+ messages in thread
From: Tomi Valkeinen @ 2016-09-23 11:33 UTC (permalink / raw)
To: Ville Syrjälä, Tomi Valkeinen; +Cc: intel-gfx, dri-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 3545 bytes --]
On 12/08/16 19:04, Ville Syrjälä wrote:
> On Thu, Aug 11, 2016 at 04:33:32PM +0300, Ville Syrjälä wrote:
>> On Thu, Aug 11, 2016 at 02:32:44PM +0300, Tomi Valkeinen wrote:
>>> Hi,
>>>
>>> On 22/07/16 16:43, ville.syrjala@linux.intel.com wrote:
>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>
>>>> The global mode_config.rotation_property is going away, switch over to
>>>> per-plane rotation_property.
>>>>
>>>> Not sure I got the annoying crtc rotation_property handling right.
>>>> Might work, or migth not.
>>>
>>> I think something is funny with this patch or the series. I fetched your
>>> branch, and with your series, it looks like the primary planes lose all
>>> their props. modetest says:
>>>
>>> could not get plane 26 properties: Invalid argument
>>> could not get plane 30 properties: Invalid argument
>>
>> Hmm. Weird. Is it really the get props ioctl that fails?
>>
>> The first EINVAL I can spot there is
>> if (!obj->properties) {
>> ret = -EINVAL;
>> goto out_unref;
>> }
>> which definitely makes no sense since this is assigned
>> as plane->base.properties = &plane->properties. So can't be that unless
>> we manage to clear the pointer somehow after the init.
>>
>> The only other direct EINVAL I see there is if
>> drm_object_property_get_value(obj->properties->properties[i])
>> fails to find the passed prop in the properties array. Which clearly
>> can't happen since we got it from the array in the first place. Also,
>> clearly that code is rather inefficient, perhaps someone should rewrite
>> it a bit.
>>
>> Can't quite see how this could fail for the plane in other ways. But I
>> might be blind.
>
> I tried to think on this a bit more, and the only think I came up with was
> that we end up doing the drm_plane_create_rotation_property() twice for the
> primary planes. I tried that on i915 but it'd didn't result in anything bad
> AFAICS. Would leak a bit, but so what :P
>
> Dunno, I guess you could try something like:
>
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -211,11 +211,12 @@ void omap_plane_install_properties(struct drm_plane *plane,
> struct omap_drm_private *priv = dev->dev_private;
>
> if (priv->has_dmm) {
> - drm_plane_create_rotation_property(plane,
> - BIT(DRM_ROTATE_0),
> - BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> - BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
> - BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
> + if (!plane->rotation_property)
> + drm_plane_create_rotation_property(plane,
> + BIT(DRM_ROTATE_0),
> + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> + BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
> + BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
This fixes the problem... Obviously something gets confused if the
property is created twice. Perhaps the first one gets stored somewhere,
and gets used somehow, even if the latter property is the "real" one,
attached to the plane? Just a guess...
Tomi
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 08/15] drm/msm/mdp5: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (6 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 07/15] drm/omap: Use per-plane rotation property ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 13:43 ` [PATCH 09/15] drm/msm/mdp5: Use per-plane rotation property ville.syrjala
` (8 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: Jilai Wang, intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
0 isn't a valid rotation property value, so let's set the initial value
of the property to BIT(DRM_ROTATE_0) instead.
In the same vein, we must always have at leat one angle as part of
set of supported rotation bits, so let's include BIT(DRM_ROTATE_0)
in there.
Cc: Rob Clark <robdclark@gmail.com>
Cc: Jilai Wang <jilaiw@codeaurora.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 432c09836b0e..cf30911f3267 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -78,12 +78,13 @@ static void mdp5_plane_install_rotation_property(struct drm_device *dev,
if (!dev->mode_config.rotation_property)
dev->mode_config.rotation_property =
drm_mode_create_rotation_property(dev,
+ BIT(DRM_ROTATE_0) |
BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
if (dev->mode_config.rotation_property)
drm_object_attach_property(&plane->base,
dev->mode_config.rotation_property,
- 0);
+ BIT(DRM_ROTATE_0));
}
/* helper to install properties which are common to planes and crtcs */
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 09/15] drm/msm/mdp5: Use per-plane rotation property
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (7 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 08/15] drm/msm/mdp5: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0 ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 13:43 ` [PATCH 10/15] drm/msm/mdp5: Advertize 180 degree rotation ville.syrjala
` (7 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: Jilai Wang, intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The global mode_config.rotation_property is going away, switch over to
per-plane rotation_property.
Cc: Rob Clark <robdclark@gmail.com>
Cc: Jilai Wang <jilaiw@codeaurora.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index cf30911f3267..2aefb0db1874 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -75,16 +75,11 @@ static void mdp5_plane_install_rotation_property(struct drm_device *dev,
!(mdp5_plane->caps & MDP_PIPE_CAP_VFLIP))
return;
- if (!dev->mode_config.rotation_property)
- dev->mode_config.rotation_property =
- drm_mode_create_rotation_property(dev,
- BIT(DRM_ROTATE_0) |
- BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
-
- if (dev->mode_config.rotation_property)
- drm_object_attach_property(&plane->base,
- dev->mode_config.rotation_property,
- BIT(DRM_ROTATE_0));
+ drm_plane_create_rotation_property(plane,
+ BIT(DRM_ROTATE_0),
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_REFLECT_X) |
+ BIT(DRM_REFLECT_Y));
}
/* helper to install properties which are common to planes and crtcs */
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 10/15] drm/msm/mdp5: Advertize 180 degree rotation
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (8 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 09/15] drm/msm/mdp5: Use per-plane rotation property ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 13:43 ` [PATCH v2 11/15] drm/i915: Use the per-plane rotation property ville.syrjala
` (6 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: Jilai Wang, intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Since the hardware can apparently do both X and Y reflection, we
can advertize also 180 degree rotation as thats just X+Y reflection.
Cc: Rob Clark <robdclark@gmail.com>
Cc: Jilai Wang <jilaiw@codeaurora.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 2aefb0db1874..586963016c89 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -78,6 +78,7 @@ static void mdp5_plane_install_rotation_property(struct drm_device *dev,
drm_plane_create_rotation_property(plane,
BIT(DRM_ROTATE_0),
BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_180) |
BIT(DRM_REFLECT_X) |
BIT(DRM_REFLECT_Y));
}
@@ -285,6 +286,8 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
plane_enabled(old_state), plane_enabled(state));
if (plane_enabled(state)) {
+ unsigned int rotation;
+
format = to_mdp_format(msm_framebuffer_format(state->fb));
if (MDP_FORMAT_IS_YUV(format) &&
!pipe_supports_yuv(mdp5_plane->caps)) {
@@ -305,8 +308,12 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}
- hflip = !!(state->rotation & BIT(DRM_REFLECT_X));
- vflip = !!(state->rotation & BIT(DRM_REFLECT_Y));
+ rotation = drm_rotation_simplify(state->rotation,
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_REFLECT_X) |
+ BIT(DRM_REFLECT_Y));
+ hflip = !!(rotation & BIT(DRM_REFLECT_X));
+ vflip = !!(rotation & BIT(DRM_REFLECT_Y));
if ((vflip && !(mdp5_plane->caps & MDP_PIPE_CAP_VFLIP)) ||
(hflip && !(mdp5_plane->caps & MDP_PIPE_CAP_HFLIP))) {
dev_err(plane->dev->dev,
@@ -677,6 +684,7 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
int pe_top[COMP_MAX], pe_bottom[COMP_MAX];
uint32_t hdecm = 0, vdecm = 0;
uint32_t pix_format;
+ unsigned int rotation;
bool vflip, hflip;
unsigned long flags;
int ret;
@@ -739,8 +747,12 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
config |= get_scale_config(format, src_h, crtc_h, false);
DBG("scale config = %x", config);
- hflip = !!(pstate->rotation & BIT(DRM_REFLECT_X));
- vflip = !!(pstate->rotation & BIT(DRM_REFLECT_Y));
+ rotation = drm_rotation_simplify(pstate->rotation,
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_REFLECT_X) |
+ BIT(DRM_REFLECT_Y));
+ hflip = !!(rotation & BIT(DRM_REFLECT_X));
+ vflip = !!(rotation & BIT(DRM_REFLECT_Y));
spin_lock_irqsave(&mdp5_plane->pipe_lock, flags);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v2 11/15] drm/i915: Use the per-plane rotation property
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (9 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 10/15] drm/msm/mdp5: Advertize 180 degree rotation ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-25 6:19 ` Joonas Lahtinen
2016-07-22 13:43 ` [PATCH 12/15] drm: RIP mode_config->rotation_property ville.syrjala
` (5 subsequent siblings)
16 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
On certain platforms not all planes support the same set of
rotations/reflections, so let's use the per-plane property
for this.
This is already a problem on SKL when we use the legay cursor plane
as it only supports 0|180 whereas the universal planes support
0|90|180|270, and it will be a problem on CHV soon.
v2: Use drm_plane_create_rotation_property() helper
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
---
drivers/gpu/drm/i915/intel_display.c | 50 +++++++++++++++---------------------
drivers/gpu/drm/i915/intel_drv.h | 3 ---
drivers/gpu/drm/i915/intel_sprite.c | 14 +++++++++-
3 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 51357986cf4a..a6d5c4d434a4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14215,6 +14215,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
struct intel_plane *primary = NULL;
struct intel_plane_state *state = NULL;
const uint32_t *intel_primary_formats;
+ unsigned int supported_rotations;
unsigned int num_formats;
int ret;
@@ -14287,8 +14288,21 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
if (ret)
goto fail;
+ if (INTEL_INFO(dev)->gen >= 9) {
+ supported_rotations =
+ BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
+ BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
+ } else if (INTEL_INFO(dev)->gen >= 4) {
+ supported_rotations =
+ BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
+ } else {
+ supported_rotations = BIT(DRM_ROTATE_0);
+ }
+
if (INTEL_INFO(dev)->gen >= 4)
- intel_create_rotation_property(dev, primary);
+ drm_plane_create_rotation_property(&primary->base,
+ BIT(DRM_ROTATE_0),
+ supported_rotations);
drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
@@ -14301,24 +14315,6 @@ fail:
return NULL;
}
-void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane)
-{
- if (!dev->mode_config.rotation_property) {
- unsigned long flags = BIT(DRM_ROTATE_0) |
- BIT(DRM_ROTATE_180);
-
- if (INTEL_INFO(dev)->gen >= 9)
- flags |= BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270);
-
- dev->mode_config.rotation_property =
- drm_mode_create_rotation_property(dev, flags);
- }
- if (dev->mode_config.rotation_property)
- drm_object_attach_property(&plane->base.base,
- dev->mode_config.rotation_property,
- plane->base.state->rotation);
-}
-
static int
intel_check_cursor_plane(struct drm_plane *plane,
struct intel_crtc_state *crtc_state,
@@ -14447,17 +14443,11 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
if (ret)
goto fail;
- if (INTEL_INFO(dev)->gen >= 4) {
- if (!dev->mode_config.rotation_property)
- dev->mode_config.rotation_property =
- drm_mode_create_rotation_property(dev,
- BIT(DRM_ROTATE_0) |
- BIT(DRM_ROTATE_180));
- if (dev->mode_config.rotation_property)
- drm_object_attach_property(&cursor->base.base,
- dev->mode_config.rotation_property,
- state->base.rotation);
- }
+ if (INTEL_INFO(dev)->gen >= 4)
+ drm_plane_create_rotation_property(&cursor->base,
+ BIT(DRM_ROTATE_0),
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_180));
if (INTEL_INFO(dev)->gen >=9)
state->scaler_id = -1;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 907a72cfdad3..9c085a3377a6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1255,9 +1255,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
uint64_t fb_modifier, unsigned int cpp);
-void intel_create_rotation_property(struct drm_device *dev,
- struct intel_plane *plane);
-
void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
enum pipe pipe);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index cc6af1410d67..a0395f8f3723 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1046,6 +1046,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
struct intel_plane_state *state = NULL;
unsigned long possible_crtcs;
const uint32_t *plane_formats;
+ unsigned int supported_rotations;
int num_plane_formats;
int ret;
@@ -1121,6 +1122,15 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
goto fail;
}
+ if (INTEL_INFO(dev)->gen >= 9) {
+ supported_rotations =
+ BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
+ BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
+ } else {
+ supported_rotations =
+ BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
+ }
+
intel_plane->pipe = pipe;
intel_plane->plane = plane;
intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
@@ -1143,7 +1153,9 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
if (ret)
goto fail;
- intel_create_rotation_property(dev, intel_plane);
+ drm_plane_create_rotation_property(&intel_plane->base,
+ BIT(DRM_ROTATE_0),
+ supported_rotations);
drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v2 11/15] drm/i915: Use the per-plane rotation property
2016-07-22 13:43 ` [PATCH v2 11/15] drm/i915: Use the per-plane rotation property ville.syrjala
@ 2016-07-25 6:19 ` Joonas Lahtinen
0 siblings, 0 replies; 41+ messages in thread
From: Joonas Lahtinen @ 2016-07-25 6:19 UTC (permalink / raw)
To: ville.syrjala, dri-devel; +Cc: intel-gfx
On pe, 2016-07-22 at 16:43 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On certain platforms not all planes support the same set of
> rotations/reflections, so let's use the per-plane property
> for this.
>
> This is already a problem on SKL when we use the legay cursor plane
> as it only supports 0|180 whereas the universal planes support
> 0|90|180|270, and it will be a problem on CHV soon.
>
> v2: Use drm_plane_create_rotation_property() helper
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 50 +++++++++++++++---------------------
> drivers/gpu/drm/i915/intel_drv.h | 3 ---
> drivers/gpu/drm/i915/intel_sprite.c | 14 +++++++++-
> 3 files changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 51357986cf4a..a6d5c4d434a4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14215,6 +14215,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> struct intel_plane *primary = NULL;
> struct intel_plane_state *state = NULL;
> const uint32_t *intel_primary_formats;
> + unsigned int supported_rotations;
> unsigned int num_formats;
> int ret;
>
> @@ -14287,8 +14288,21 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> if (ret)
> goto fail;
>
> + if (INTEL_INFO(dev)->gen >= 9) {
> + supported_rotations =
> + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> + BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
> + } else if (INTEL_INFO(dev)->gen >= 4) {
> + supported_rotations =
> + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
> + } else {
> + supported_rotations = BIT(DRM_ROTATE_0);
> + }
> +
> if (INTEL_INFO(dev)->gen >= 4)
> - intel_create_rotation_property(dev, primary);
> + drm_plane_create_rotation_property(&primary->base,
> + BIT(DRM_ROTATE_0),
> + supported_rotations);
>
> drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
>
> @@ -14301,24 +14315,6 @@ fail:
> return NULL;
> }
>
> -void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane)
> -{
> - if (!dev->mode_config.rotation_property) {
> - unsigned long flags = BIT(DRM_ROTATE_0) |
> - BIT(DRM_ROTATE_180);
> -
> - if (INTEL_INFO(dev)->gen >= 9)
> - flags |= BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270);
> -
> - dev->mode_config.rotation_property =
> - drm_mode_create_rotation_property(dev, flags);
> - }
> - if (dev->mode_config.rotation_property)
> - drm_object_attach_property(&plane->base.base,
> - dev->mode_config.rotation_property,
> - plane->base.state->rotation);
> -}
> -
> static int
> intel_check_cursor_plane(struct drm_plane *plane,
> struct intel_crtc_state *crtc_state,
> @@ -14447,17 +14443,11 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> if (ret)
> goto fail;
>
> - if (INTEL_INFO(dev)->gen >= 4) {
> - if (!dev->mode_config.rotation_property)
> - dev->mode_config.rotation_property =
> - drm_mode_create_rotation_property(dev,
> - BIT(DRM_ROTATE_0) |
> - BIT(DRM_ROTATE_180));
> - if (dev->mode_config.rotation_property)
> - drm_object_attach_property(&cursor->base.base,
> - dev->mode_config.rotation_property,
> - state->base.rotation);
> - }
> + if (INTEL_INFO(dev)->gen >= 4)
> + drm_plane_create_rotation_property(&cursor->base,
> + BIT(DRM_ROTATE_0),
> + BIT(DRM_ROTATE_0) |
> + BIT(DRM_ROTATE_180));
>
> if (INTEL_INFO(dev)->gen >=9)
> state->scaler_id = -1;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 907a72cfdad3..9c085a3377a6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1255,9 +1255,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
> uint64_t fb_modifier, unsigned int cpp);
>
> -void intel_create_rotation_property(struct drm_device *dev,
> - struct intel_plane *plane);
> -
> void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
> enum pipe pipe);
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index cc6af1410d67..a0395f8f3723 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1046,6 +1046,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> struct intel_plane_state *state = NULL;
> unsigned long possible_crtcs;
> const uint32_t *plane_formats;
> + unsigned int supported_rotations;
> int num_plane_formats;
> int ret;
>
> @@ -1121,6 +1122,15 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> goto fail;
> }
>
> + if (INTEL_INFO(dev)->gen >= 9) {
> + supported_rotations =
> + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> + BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
> + } else {
> + supported_rotations =
> + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
> + }
> +
> intel_plane->pipe = pipe;
> intel_plane->plane = plane;
> intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
> @@ -1143,7 +1153,9 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> if (ret)
> goto fail;
>
> - intel_create_rotation_property(dev, intel_plane);
> + drm_plane_create_rotation_property(&intel_plane->base,
> + BIT(DRM_ROTATE_0),
> + supported_rotations);
>
> drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 12/15] drm: RIP mode_config->rotation_property
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (10 preceding siblings ...)
2016-07-22 13:43 ` [PATCH v2 11/15] drm/i915: Use the per-plane rotation property ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-25 6:08 ` Joonas Lahtinen
2016-10-17 22:38 ` Laurent Pinchart
2016-07-22 13:43 ` [PATCH 13/15] drm/i915: Use & instead if == to check for rotations ville.syrjala
` (4 subsequent siblings)
16 siblings, 2 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Now that all drivers have been converted over to the per-plane rotation
property, we can just nuke the global rotation property.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 6 ++----
drivers/gpu/drm/drm_crtc.c | 18 ------------------
drivers/gpu/drm/drm_fb_helper.c | 7 +------
include/drm/drm_crtc.h | 7 -------
4 files changed, 3 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 116f940a9267..81061fcdb984 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -709,8 +709,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
state->src_w = val;
} else if (property == config->prop_src_h) {
state->src_h = val;
- } else if (property == config->rotation_property ||
- property == plane->rotation_property) {
+ } else if (property == plane->rotation_property) {
if (!is_power_of_2(val & DRM_ROTATE_MASK))
return -EINVAL;
state->rotation = val;
@@ -768,8 +767,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_w;
} else if (property == config->prop_src_h) {
*val = state->src_h;
- } else if (property == config->rotation_property ||
- property == plane->rotation_property) {
+ } else if (property == plane->rotation_property) {
*val = state->rotation;
} else if (plane->funcs->atomic_get_property) {
return plane->funcs->atomic_get_property(plane, state, property, val);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9e20a52ece7c..c1df75caf72f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5783,24 +5783,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_config_cleanup);
-struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
- unsigned int supported_rotations)
-{
- static const struct drm_prop_enum_list props[] = {
- { DRM_ROTATE_0, "rotate-0" },
- { DRM_ROTATE_90, "rotate-90" },
- { DRM_ROTATE_180, "rotate-180" },
- { DRM_ROTATE_270, "rotate-270" },
- { DRM_REFLECT_X, "reflect-x" },
- { DRM_REFLECT_Y, "reflect-y" },
- };
-
- return drm_property_create_bitmask(dev, 0, "rotation",
- props, ARRAY_SIZE(props),
- supported_rotations);
-}
-EXPORT_SYMBOL(drm_mode_create_rotation_property);
-
int drm_plane_create_rotation_property(struct drm_plane *plane,
unsigned int rotation,
unsigned int supported_rotations)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index ce536c0553e5..cf5f071ffae1 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -392,15 +392,10 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
if (plane->type != DRM_PLANE_TYPE_PRIMARY)
drm_plane_force_disable(plane);
- if (plane->rotation_property) {
+ if (plane->rotation_property)
drm_mode_plane_set_obj_prop(plane,
plane->rotation_property,
BIT(DRM_ROTATE_0));
- } else if (dev->mode_config.rotation_property) {
- drm_mode_plane_set_obj_prop(plane,
- dev->mode_config.rotation_property,
- BIT(DRM_ROTATE_0));
- }
}
for (i = 0; i < fb_helper->crtc_count; i++) {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 01cf0673f6c8..00a93e44f854 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2480,11 +2480,6 @@ struct drm_mode_config {
*/
struct drm_property *plane_type_property;
/**
- * @rotation_property: Optional property for planes or CRTCs to specify
- * rotation.
- */
- struct drm_property *rotation_property;
- /**
* @prop_src_x: Default atomic plane property for the plane source
* position in the connected &drm_framebuffer.
*/
@@ -2960,8 +2955,6 @@ extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
struct drm_property *property,
uint64_t value);
-extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
- unsigned int supported_rotations);
extern int drm_plane_create_rotation_property(struct drm_plane *plane,
unsigned int rotation,
unsigned int supported_rotations);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 12/15] drm: RIP mode_config->rotation_property
2016-07-22 13:43 ` [PATCH 12/15] drm: RIP mode_config->rotation_property ville.syrjala
@ 2016-07-25 6:08 ` Joonas Lahtinen
2016-10-17 22:38 ` Laurent Pinchart
1 sibling, 0 replies; 41+ messages in thread
From: Joonas Lahtinen @ 2016-07-25 6:08 UTC (permalink / raw)
To: ville.syrjala, dri-devel; +Cc: intel-gfx
On pe, 2016-07-22 at 16:43 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that all drivers have been converted over to the per-plane rotation
> property, we can just nuke the global rotation property.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 6 ++----
> drivers/gpu/drm/drm_crtc.c | 18 ------------------
> drivers/gpu/drm/drm_fb_helper.c | 7 +------
> include/drm/drm_crtc.h | 7 -------
> 4 files changed, 3 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 116f940a9267..81061fcdb984 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -709,8 +709,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
> state->src_w = val;
> } else if (property == config->prop_src_h) {
> state->src_h = val;
> - } else if (property == config->rotation_property ||
> - property == plane->rotation_property) {
> + } else if (property == plane->rotation_property) {
> if (!is_power_of_2(val & DRM_ROTATE_MASK))
> return -EINVAL;
> state->rotation = val;
> @@ -768,8 +767,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->src_w;
> } else if (property == config->prop_src_h) {
> *val = state->src_h;
> - } else if (property == config->rotation_property ||
> - property == plane->rotation_property) {
> + } else if (property == plane->rotation_property) {
> *val = state->rotation;
> } else if (plane->funcs->atomic_get_property) {
> return plane->funcs->atomic_get_property(plane, state, property, val);
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 9e20a52ece7c..c1df75caf72f 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5783,24 +5783,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_mode_config_cleanup);
>
> -struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> - unsigned int supported_rotations)
> -{
> - static const struct drm_prop_enum_list props[] = {
> - { DRM_ROTATE_0, "rotate-0" },
> - { DRM_ROTATE_90, "rotate-90" },
> - { DRM_ROTATE_180, "rotate-180" },
> - { DRM_ROTATE_270, "rotate-270" },
> - { DRM_REFLECT_X, "reflect-x" },
> - { DRM_REFLECT_Y, "reflect-y" },
> - };
> -
> - return drm_property_create_bitmask(dev, 0, "rotation",
> - props, ARRAY_SIZE(props),
> - supported_rotations);
> -}
> -EXPORT_SYMBOL(drm_mode_create_rotation_property);
> -
> int drm_plane_create_rotation_property(struct drm_plane *plane,
> unsigned int rotation,
> unsigned int supported_rotations)
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index ce536c0553e5..cf5f071ffae1 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -392,15 +392,10 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> drm_plane_force_disable(plane);
>
> - if (plane->rotation_property) {
> + if (plane->rotation_property)
> drm_mode_plane_set_obj_prop(plane,
> plane->rotation_property,
> BIT(DRM_ROTATE_0));
> - } else if (dev->mode_config.rotation_property) {
> - drm_mode_plane_set_obj_prop(plane,
> - dev->mode_config.rotation_property,
> - BIT(DRM_ROTATE_0));
> - }
> }
>
> for (i = 0; i < fb_helper->crtc_count; i++) {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 01cf0673f6c8..00a93e44f854 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2480,11 +2480,6 @@ struct drm_mode_config {
> */
> struct drm_property *plane_type_property;
> /**
> - * @rotation_property: Optional property for planes or CRTCs to specify
> - * rotation.
> - */
> - struct drm_property *rotation_property;
> - /**
> * @prop_src_x: Default atomic plane property for the plane source
> * position in the connected &drm_framebuffer.
> */
> @@ -2960,8 +2955,6 @@ extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
> struct drm_property *property,
> uint64_t value);
>
> -extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> - unsigned int supported_rotations);
> extern int drm_plane_create_rotation_property(struct drm_plane *plane,
> unsigned int rotation,
> unsigned int supported_rotations);
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 12/15] drm: RIP mode_config->rotation_property
2016-07-22 13:43 ` [PATCH 12/15] drm: RIP mode_config->rotation_property ville.syrjala
2016-07-25 6:08 ` Joonas Lahtinen
@ 2016-10-17 22:38 ` Laurent Pinchart
2016-10-18 7:36 ` Daniel Vetter
1 sibling, 1 reply; 41+ messages in thread
From: Laurent Pinchart @ 2016-10-17 22:38 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
Hi Ville,
On Friday 22 Jul 2016 16:43:13 ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that all drivers have been converted over to the per-plane rotation
> property, we can just nuke the global rotation property.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Stupid question, but how does this work when the hardware supports global
rotation only, not per-plane rotation ?
> ---
> drivers/gpu/drm/drm_atomic.c | 6 ++----
> drivers/gpu/drm/drm_crtc.c | 18 ------------------
> drivers/gpu/drm/drm_fb_helper.c | 7 +------
> include/drm/drm_crtc.h | 7 -------
> 4 files changed, 3 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 116f940a9267..81061fcdb984 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -709,8 +709,7 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane, state->src_w = val;
> } else if (property == config->prop_src_h) {
> state->src_h = val;
> - } else if (property == config->rotation_property ||
> - property == plane->rotation_property) {
> + } else if (property == plane->rotation_property) {
> if (!is_power_of_2(val & DRM_ROTATE_MASK))
> return -EINVAL;
> state->rotation = val;
> @@ -768,8 +767,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->src_w;
> } else if (property == config->prop_src_h) {
> *val = state->src_h;
> - } else if (property == config->rotation_property ||
> - property == plane->rotation_property) {
> + } else if (property == plane->rotation_property) {
> *val = state->rotation;
> } else if (plane->funcs->atomic_get_property) {
> return plane->funcs->atomic_get_property(plane, state,
property, val);
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 9e20a52ece7c..c1df75caf72f 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5783,24 +5783,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_mode_config_cleanup);
>
> -struct drm_property *drm_mode_create_rotation_property(struct drm_device
> *dev, - unsigned int
supported_rotations)
> -{
> - static const struct drm_prop_enum_list props[] = {
> - { DRM_ROTATE_0, "rotate-0" },
> - { DRM_ROTATE_90, "rotate-90" },
> - { DRM_ROTATE_180, "rotate-180" },
> - { DRM_ROTATE_270, "rotate-270" },
> - { DRM_REFLECT_X, "reflect-x" },
> - { DRM_REFLECT_Y, "reflect-y" },
> - };
> -
> - return drm_property_create_bitmask(dev, 0, "rotation",
> - props, ARRAY_SIZE(props),
> - supported_rotations);
> -}
> -EXPORT_SYMBOL(drm_mode_create_rotation_property);
> -
> int drm_plane_create_rotation_property(struct drm_plane *plane,
> unsigned int rotation,
> unsigned int supported_rotations)
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c index ce536c0553e5..cf5f071ffae1 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -392,15 +392,10 @@ static int restore_fbdev_mode(struct drm_fb_helper
> *fb_helper) if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> drm_plane_force_disable(plane);
>
> - if (plane->rotation_property) {
> + if (plane->rotation_property)
> drm_mode_plane_set_obj_prop(plane,
> plane->rotation_property,
> BIT(DRM_ROTATE_0));
> - } else if (dev->mode_config.rotation_property) {
> - drm_mode_plane_set_obj_prop(plane,
> - dev-
>mode_config.rotation_property,
> - BIT(DRM_ROTATE_0));
> - }
> }
>
> for (i = 0; i < fb_helper->crtc_count; i++) {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 01cf0673f6c8..00a93e44f854 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2480,11 +2480,6 @@ struct drm_mode_config {
> */
> struct drm_property *plane_type_property;
> /**
> - * @rotation_property: Optional property for planes or CRTCs to
specify
> - * rotation.
> - */
> - struct drm_property *rotation_property;
> - /**
> * @prop_src_x: Default atomic plane property for the plane source
> * position in the connected &drm_framebuffer.
> */
> @@ -2960,8 +2955,6 @@ extern int drm_mode_plane_set_obj_prop(struct
> drm_plane *plane, struct drm_property *property,
> uint64_t value);
>
> -extern struct drm_property *drm_mode_create_rotation_property(struct
> drm_device *dev, -
unsigned int supported_rotations);
> extern int drm_plane_create_rotation_property(struct drm_plane *plane,
> unsigned int rotation,
> unsigned int
supported_rotations);
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 12/15] drm: RIP mode_config->rotation_property
2016-10-17 22:38 ` Laurent Pinchart
@ 2016-10-18 7:36 ` Daniel Vetter
2016-10-18 8:13 ` Laurent Pinchart
0 siblings, 1 reply; 41+ messages in thread
From: Daniel Vetter @ 2016-10-18 7:36 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: intel-gfx, dri-devel
On Tue, Oct 18, 2016 at 01:38:05AM +0300, Laurent Pinchart wrote:
> Hi Ville,
>
> On Friday 22 Jul 2016 16:43:13 ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Now that all drivers have been converted over to the per-plane rotation
> > property, we can just nuke the global rotation property.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Stupid question, but how does this work when the hardware supports global
> rotation only, not per-plane rotation ?
Does that exist? If so I guess we get to add a rotation property to CRTCS
(and update docs and all that).
-Daniel
>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 6 ++----
> > drivers/gpu/drm/drm_crtc.c | 18 ------------------
> > drivers/gpu/drm/drm_fb_helper.c | 7 +------
> > include/drm/drm_crtc.h | 7 -------
> > 4 files changed, 3 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 116f940a9267..81061fcdb984 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -709,8 +709,7 @@ int drm_atomic_plane_set_property(struct drm_plane
> > *plane, state->src_w = val;
> > } else if (property == config->prop_src_h) {
> > state->src_h = val;
> > - } else if (property == config->rotation_property ||
> > - property == plane->rotation_property) {
> > + } else if (property == plane->rotation_property) {
> > if (!is_power_of_2(val & DRM_ROTATE_MASK))
> > return -EINVAL;
> > state->rotation = val;
> > @@ -768,8 +767,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> > *val = state->src_w;
> > } else if (property == config->prop_src_h) {
> > *val = state->src_h;
> > - } else if (property == config->rotation_property ||
> > - property == plane->rotation_property) {
> > + } else if (property == plane->rotation_property) {
> > *val = state->rotation;
> > } else if (plane->funcs->atomic_get_property) {
> > return plane->funcs->atomic_get_property(plane, state,
> property, val);
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 9e20a52ece7c..c1df75caf72f 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -5783,24 +5783,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> > }
> > EXPORT_SYMBOL(drm_mode_config_cleanup);
> >
> > -struct drm_property *drm_mode_create_rotation_property(struct drm_device
> > *dev, - unsigned int
> supported_rotations)
> > -{
> > - static const struct drm_prop_enum_list props[] = {
> > - { DRM_ROTATE_0, "rotate-0" },
> > - { DRM_ROTATE_90, "rotate-90" },
> > - { DRM_ROTATE_180, "rotate-180" },
> > - { DRM_ROTATE_270, "rotate-270" },
> > - { DRM_REFLECT_X, "reflect-x" },
> > - { DRM_REFLECT_Y, "reflect-y" },
> > - };
> > -
> > - return drm_property_create_bitmask(dev, 0, "rotation",
> > - props, ARRAY_SIZE(props),
> > - supported_rotations);
> > -}
> > -EXPORT_SYMBOL(drm_mode_create_rotation_property);
> > -
> > int drm_plane_create_rotation_property(struct drm_plane *plane,
> > unsigned int rotation,
> > unsigned int supported_rotations)
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > b/drivers/gpu/drm/drm_fb_helper.c index ce536c0553e5..cf5f071ffae1 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -392,15 +392,10 @@ static int restore_fbdev_mode(struct drm_fb_helper
> > *fb_helper) if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> > drm_plane_force_disable(plane);
> >
> > - if (plane->rotation_property) {
> > + if (plane->rotation_property)
> > drm_mode_plane_set_obj_prop(plane,
> > plane->rotation_property,
> > BIT(DRM_ROTATE_0));
> > - } else if (dev->mode_config.rotation_property) {
> > - drm_mode_plane_set_obj_prop(plane,
> > - dev-
> >mode_config.rotation_property,
> > - BIT(DRM_ROTATE_0));
> > - }
> > }
> >
> > for (i = 0; i < fb_helper->crtc_count; i++) {
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 01cf0673f6c8..00a93e44f854 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -2480,11 +2480,6 @@ struct drm_mode_config {
> > */
> > struct drm_property *plane_type_property;
> > /**
> > - * @rotation_property: Optional property for planes or CRTCs to
> specify
> > - * rotation.
> > - */
> > - struct drm_property *rotation_property;
> > - /**
> > * @prop_src_x: Default atomic plane property for the plane source
> > * position in the connected &drm_framebuffer.
> > */
> > @@ -2960,8 +2955,6 @@ extern int drm_mode_plane_set_obj_prop(struct
> > drm_plane *plane, struct drm_property *property,
> > uint64_t value);
> >
> > -extern struct drm_property *drm_mode_create_rotation_property(struct
> > drm_device *dev, -
> unsigned int supported_rotations);
> > extern int drm_plane_create_rotation_property(struct drm_plane *plane,
> > unsigned int rotation,
> > unsigned int
> supported_rotations);
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 12/15] drm: RIP mode_config->rotation_property
2016-10-18 7:36 ` Daniel Vetter
@ 2016-10-18 8:13 ` Laurent Pinchart
2016-10-18 9:16 ` [Intel-gfx] " Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Laurent Pinchart @ 2016-10-18 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
Hi Daniel,
On Tuesday 18 Oct 2016 09:36:23 Daniel Vetter wrote:
> On Tue, Oct 18, 2016 at 01:38:05AM +0300, Laurent Pinchart wrote:
> > On Friday 22 Jul 2016 16:43:13 ville.syrjala@linux.intel.com wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Now that all drivers have been converted over to the per-plane rotation
> >> property, we can just nuke the global rotation property.
> >>
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Stupid question, but how does this work when the hardware supports global
> > rotation only, not per-plane rotation ?
>
> Does that exist? If so I guess we get to add a rotation property to CRTCS
> (and update docs and all that).
I have a hardware composer that performs rotation on the output side. At the
moment it doesn't support rotation when used with the display, but I wouldn't
rule that out in future SoC versions. So I'm not aware of any use case for a
rotation property on the CRTC at the moment, I just wanted to make sure we
have a path forward.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 12/15] drm: RIP mode_config->rotation_property
2016-10-18 8:13 ` Laurent Pinchart
@ 2016-10-18 9:16 ` Ville Syrjälä
0 siblings, 0 replies; 41+ messages in thread
From: Ville Syrjälä @ 2016-10-18 9:16 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: intel-gfx, dri-devel
On Tue, Oct 18, 2016 at 11:13:53AM +0300, Laurent Pinchart wrote:
> Hi Daniel,
>
> On Tuesday 18 Oct 2016 09:36:23 Daniel Vetter wrote:
> > On Tue, Oct 18, 2016 at 01:38:05AM +0300, Laurent Pinchart wrote:
> > > On Friday 22 Jul 2016 16:43:13 ville.syrjala@linux.intel.com wrote:
> > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>
> > >> Now that all drivers have been converted over to the per-plane rotation
> > >> property, we can just nuke the global rotation property.
> > >>
> > >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Stupid question, but how does this work when the hardware supports global
> > > rotation only, not per-plane rotation ?
> >
> > Does that exist? If so I guess we get to add a rotation property to CRTCS
> > (and update docs and all that).
>
> I have a hardware composer that performs rotation on the output side. At the
> moment it doesn't support rotation when used with the display, but I wouldn't
> rule that out in future SoC versions. So I'm not aware of any use case for a
> rotation property on the CRTC at the moment, I just wanted to make sure we
> have a path forward.
I even implemented that for i915 long ago by having it rotate all the
planes (+ adjust their coordinates appropriately). But people weren't
too keen on putting that in, so I dropped it.
https://lists.freedesktop.org/archives/intel-gfx/2014-February/040053.html
The only bigger snafu here is omap, which already has a rotation property
on the crtc, except it actually only rotates the primary plane. So
when/if someone adds a real crtc rotation property it has to be called
something else that "rotation". I think I settled on "crtc-rotation"
judging what I have in my last branch on that topic.
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 13/15] drm/i915: Use & instead if == to check for rotations
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (11 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 12/15] drm: RIP mode_config->rotation_property ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 13:43 ` [PATCH 14/15] drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup ville.syrjala
` (3 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Using == to check for 180 degree rotation only works as long as the
reflection bits aren't set. That will change soon enough for CHV, so
let's stop doing things the wrong way.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 9 +++++----
drivers/gpu/drm/i915/intel_sprite.c | 6 +++---
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a6d5c4d434a4..5f36040eb96b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2688,7 +2688,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
intel_crtc->dspaddr_offset = linear_offset;
}
- if (rotation == BIT(DRM_ROTATE_180)) {
+ if (rotation & BIT(DRM_ROTATE_180)) {
dspcntr |= DISPPLANE_ROTATE_180;
x += (crtc_state->pipe_src_w - 1);
@@ -2791,7 +2791,8 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
intel_compute_tile_offset(&x, &y, fb, 0,
fb->pitches[0], rotation);
linear_offset -= intel_crtc->dspaddr_offset;
- if (rotation == BIT(DRM_ROTATE_180)) {
+
+ if (rotation & BIT(DRM_ROTATE_180)) {
dspcntr |= DISPPLANE_ROTATE_180;
if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
@@ -10263,7 +10264,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
if (HAS_DDI(dev))
cntl |= CURSOR_PIPE_CSC_ENABLE;
- if (plane_state->base.rotation == BIT(DRM_ROTATE_180))
+ if (plane_state->base.rotation & BIT(DRM_ROTATE_180))
cntl |= CURSOR_ROTATE_180;
}
@@ -10309,7 +10310,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
/* ILK+ do this automagically */
if (HAS_GMCH_DISPLAY(dev) &&
- plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
+ plane_state->base.rotation & BIT(DRM_ROTATE_180)) {
base += (plane_state->base.crtc_h *
plane_state->base.crtc_w - 1) * 4;
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a0395f8f3723..ccb894f39425 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -444,7 +444,7 @@ vlv_update_plane(struct drm_plane *dplane,
fb->pitches[0], rotation);
linear_offset -= sprsurf_offset;
- if (rotation == BIT(DRM_ROTATE_180)) {
+ if (rotation & BIT(DRM_ROTATE_180)) {
sprctl |= SP_ROTATE_180;
x += src_w;
@@ -577,7 +577,7 @@ ivb_update_plane(struct drm_plane *plane,
fb->pitches[0], rotation);
linear_offset -= sprsurf_offset;
- if (rotation == BIT(DRM_ROTATE_180)) {
+ if (rotation & BIT(DRM_ROTATE_180)) {
sprctl |= SPRITE_ROTATE_180;
/* HSW and BDW does this automagically in hardware */
@@ -714,7 +714,7 @@ ilk_update_plane(struct drm_plane *plane,
fb->pitches[0], rotation);
linear_offset -= dvssurf_offset;
- if (rotation == BIT(DRM_ROTATE_180)) {
+ if (rotation & BIT(DRM_ROTATE_180)) {
dvscntr |= DVS_ROTATE_180;
x += src_w;
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 14/15] drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (12 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 13/15] drm/i915: Use & instead if == to check for rotations ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 13:43 ` [PATCH 15/15] drm/i915: Add horizontal mirroring support for CHV pipe B planes ville.syrjala
` (2 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Move the plane control register rotation setup away from the
coordinate munging code. This will result in neater looking
code once we add reflection support for CHV.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++--------------
drivers/gpu/drm/i915/intel_sprite.c | 28 +++++++++++++++-------------
2 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5f36040eb96b..5c612a0dffb3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2674,6 +2674,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
obj->tiling_mode != I915_TILING_NONE)
dspcntr |= DISPPLANE_TILED;
+ if (rotation & BIT(DRM_ROTATE_180))
+ dspcntr |= DISPPLANE_ROTATE_180;
+
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
@@ -2689,8 +2692,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
}
if (rotation & BIT(DRM_ROTATE_180)) {
- dspcntr |= DISPPLANE_ROTATE_180;
-
x += (crtc_state->pipe_src_w - 1);
y += (crtc_state->pipe_src_h - 1);
@@ -2783,6 +2784,9 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
if (obj->tiling_mode != I915_TILING_NONE)
dspcntr |= DISPPLANE_TILED;
+ if (rotation & BIT(DRM_ROTATE_180))
+ dspcntr |= DISPPLANE_ROTATE_180;
+
if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
@@ -2792,19 +2796,15 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
fb->pitches[0], rotation);
linear_offset -= intel_crtc->dspaddr_offset;
- if (rotation & BIT(DRM_ROTATE_180)) {
- dspcntr |= DISPPLANE_ROTATE_180;
-
- if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
- x += (crtc_state->pipe_src_w - 1);
- y += (crtc_state->pipe_src_h - 1);
+ /* HSW and BDW does this automagically in hardware */
+ if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) &&
+ rotation & BIT(DRM_ROTATE_180)) {
+ x += (crtc_state->pipe_src_w - 1);
+ y += (crtc_state->pipe_src_h - 1);
- /* Finding the last pixel of the last line of the display
- data and adding to linear_offset*/
- linear_offset +=
- (crtc_state->pipe_src_h - 1) * fb->pitches[0] +
- (crtc_state->pipe_src_w - 1) * cpp;
- }
+ linear_offset +=
+ (crtc_state->pipe_src_h - 1) * fb->pitches[0] +
+ (crtc_state->pipe_src_w - 1) * cpp;
}
intel_crtc->adjusted_x = x;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index ccb894f39425..f26594e2e851 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -433,6 +433,9 @@ vlv_update_plane(struct drm_plane *dplane,
if (obj->tiling_mode != I915_TILING_NONE)
sprctl |= SP_TILED;
+ if (rotation & BIT(DRM_ROTATE_180))
+ sprctl |= SP_ROTATE_180;
+
/* Sizes are 0 based */
src_w--;
src_h--;
@@ -445,8 +448,6 @@ vlv_update_plane(struct drm_plane *dplane,
linear_offset -= sprsurf_offset;
if (rotation & BIT(DRM_ROTATE_180)) {
- sprctl |= SP_ROTATE_180;
-
x += src_w;
y += src_h;
linear_offset += src_h * fb->pitches[0] + src_w * cpp;
@@ -555,6 +556,9 @@ ivb_update_plane(struct drm_plane *plane,
if (obj->tiling_mode != I915_TILING_NONE)
sprctl |= SPRITE_TILED;
+ if (rotation & BIT(DRM_ROTATE_180))
+ sprctl |= SPRITE_ROTATE_180;
+
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
sprctl &= ~SPRITE_TRICKLE_FEED_DISABLE;
else
@@ -577,15 +581,12 @@ ivb_update_plane(struct drm_plane *plane,
fb->pitches[0], rotation);
linear_offset -= sprsurf_offset;
- if (rotation & BIT(DRM_ROTATE_180)) {
- sprctl |= SPRITE_ROTATE_180;
-
- /* HSW and BDW does this automagically in hardware */
- if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
- x += src_w;
- y += src_h;
- linear_offset += src_h * fb->pitches[0] + src_w * cpp;
- }
+ /* HSW and BDW does this automagically in hardware */
+ if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) &&
+ rotation & BIT(DRM_ROTATE_180)) {
+ x += src_w;
+ y += src_h;
+ linear_offset += src_h * fb->pitches[0] + src_w * cpp;
}
if (key->flags) {
@@ -696,6 +697,9 @@ ilk_update_plane(struct drm_plane *plane,
if (obj->tiling_mode != I915_TILING_NONE)
dvscntr |= DVS_TILED;
+ if (rotation & BIT(DRM_ROTATE_180))
+ dvscntr |= DVS_ROTATE_180;
+
if (IS_GEN6(dev))
dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
@@ -715,8 +719,6 @@ ilk_update_plane(struct drm_plane *plane,
linear_offset -= dvssurf_offset;
if (rotation & BIT(DRM_ROTATE_180)) {
- dvscntr |= DVS_ROTATE_180;
-
x += src_w;
y += src_h;
linear_offset += src_h * fb->pitches[0] + src_w * cpp;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 15/15] drm/i915: Add horizontal mirroring support for CHV pipe B planes
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (13 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 14/15] drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup ville.syrjala
@ 2016-07-22 13:43 ` ville.syrjala
2016-07-22 14:24 ` ✗ Ro.CI.BAT: failure for drm: Per-plane rotation etc Patchwork
2016-07-22 16:31 ` ✗ Ro.CI.BAT: failure for drm: Per-plane rotation etc. (rev2) Patchwork
16 siblings, 0 replies; 41+ messages in thread
From: ville.syrjala @ 2016-07-22 13:43 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The primary and sprite planes on CHV pipe B support horizontal
mirroring. Expose it to the world.
Sadly the hardware ignores the mirror bit when the rotate bit is
set, so we'll have to reject the 180+X case.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 8 ++++++++
drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
drivers/gpu/drm/i915/intel_sprite.c | 10 ++++++++++
3 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index c19eb9a0cd4a..0a019eacfede 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -180,6 +180,14 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
}
}
+ /* CHV ignores the mirror bit when the rotate bit is set :( */
+ if (IS_CHERRYVIEW(plane->dev) &&
+ state->rotation & BIT(DRM_ROTATE_180) &&
+ state->rotation & BIT(DRM_REFLECT_X)) {
+ DRM_DEBUG_KMS("Cannot rotate and reflect at the same time\n");
+ return -EINVAL;
+ }
+
intel_state->visible = false;
ret = intel_plane->check_plane(plane, crtc_state, intel_state);
if (ret)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5c612a0dffb3..9d803982554b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2677,6 +2677,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
if (rotation & BIT(DRM_ROTATE_180))
dspcntr |= DISPPLANE_ROTATE_180;
+ if (rotation & BIT(DRM_REFLECT_X))
+ dspcntr |= DISPPLANE_MIRROR;
+
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
@@ -2700,6 +2703,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
linear_offset +=
(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
(crtc_state->pipe_src_w - 1) * cpp;
+ } else if (rotation & BIT(DRM_REFLECT_X)) {
+ x += (crtc_state->pipe_src_w - 1);
+ linear_offset += (crtc_state->pipe_src_w - 1) * cpp;
}
intel_crtc->adjusted_x = x;
@@ -14293,6 +14299,10 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
supported_rotations =
BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
+ } else if (IS_CHERRYVIEW(dev) && pipe == PIPE_B) {
+ supported_rotations =
+ BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180) |
+ BIT(DRM_REFLECT_X);
} else if (INTEL_INFO(dev)->gen >= 4) {
supported_rotations =
BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index f26594e2e851..2e2063633ca9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -436,6 +436,9 @@ vlv_update_plane(struct drm_plane *dplane,
if (rotation & BIT(DRM_ROTATE_180))
sprctl |= SP_ROTATE_180;
+ if (rotation & BIT(DRM_REFLECT_X))
+ sprctl |= SP_MIRROR;
+
/* Sizes are 0 based */
src_w--;
src_h--;
@@ -451,6 +454,9 @@ vlv_update_plane(struct drm_plane *dplane,
x += src_w;
y += src_h;
linear_offset += src_h * fb->pitches[0] + src_w * cpp;
+ } else if (rotation & BIT(DRM_REFLECT_X)) {
+ x += src_w;
+ linear_offset += src_w * cpp;
}
if (key->flags) {
@@ -1128,6 +1134,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
supported_rotations =
BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
+ } else if (IS_CHERRYVIEW(dev) && pipe == PIPE_B) {
+ supported_rotations =
+ BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180) |
+ BIT(DRM_REFLECT_X);
} else {
supported_rotations =
BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* ✗ Ro.CI.BAT: failure for drm: Per-plane rotation etc.
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (14 preceding siblings ...)
2016-07-22 13:43 ` [PATCH 15/15] drm/i915: Add horizontal mirroring support for CHV pipe B planes ville.syrjala
@ 2016-07-22 14:24 ` Patchwork
2016-07-22 16:31 ` ✗ Ro.CI.BAT: failure for drm: Per-plane rotation etc. (rev2) Patchwork
16 siblings, 0 replies; 41+ messages in thread
From: Patchwork @ 2016-07-22 14:24 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm: Per-plane rotation etc.
URL : https://patchwork.freedesktop.org/series/10189/
State : failure
== Summary ==
Series 10189v1 drm: Per-plane rotation etc.
http://patchwork.freedesktop.org/api/1.0/series/10189/revisions/1/mbox
Test gem_sync:
Subgroup basic-store-each:
pass -> DMESG-FAIL (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> INCOMPLETE (fi-hsw-i7-4770k)
fi-hsw-i7-4770k total:202 pass:183 dwarn:0 dfail:0 fail:0 skip:18
fi-kbl-qkkr total:244 pass:179 dwarn:27 dfail:0 fail:10 skip:28
fi-skl-i5-6260u total:244 pass:224 dwarn:0 dfail:0 fail:8 skip:12
fi-skl-i7-6700k total:244 pass:210 dwarn:0 dfail:0 fail:8 skip:26
fi-snb-i7-2600 total:244 pass:196 dwarn:0 dfail:0 fail:8 skip:40
ro-bdw-i5-5250u total:244 pass:219 dwarn:4 dfail:0 fail:8 skip:13
ro-bdw-i7-5557U total:244 pass:220 dwarn:3 dfail:0 fail:8 skip:13
ro-bdw-i7-5600u total:244 pass:203 dwarn:0 dfail:1 fail:8 skip:32
ro-bsw-n3050 total:218 pass:173 dwarn:0 dfail:0 fail:2 skip:42
ro-byt-n2820 total:244 pass:197 dwarn:0 dfail:0 fail:9 skip:38
ro-hsw-i3-4010u total:244 pass:212 dwarn:0 dfail:0 fail:8 skip:24
ro-hsw-i7-4770r total:244 pass:212 dwarn:0 dfail:0 fail:8 skip:24
ro-ilk-i7-620lm total:244 pass:172 dwarn:0 dfail:0 fail:9 skip:63
ro-ilk1-i5-650 total:239 pass:172 dwarn:0 dfail:0 fail:9 skip:58
ro-ivb-i7-3770 total:244 pass:203 dwarn:0 dfail:0 fail:8 skip:33
ro-skl3-i5-6260u total:244 pass:224 dwarn:0 dfail:0 fail:8 skip:12
ro-snb-i7-2620M total:244 pass:193 dwarn:0 dfail:0 fail:9 skip:42
Results at /archive/results/CI_IGT_test/RO_Patchwork_1579/
7df4039 drm-intel-nightly: 2016y-07m-22d-09h-32m-51s UTC integration manifest
5637f2b drm/i915: Add horizontal mirroring support for CHV pipe B planes
e9b4911 drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup
a56d82a drm/i915: Use & instead if == to check for rotations
7767153 drm: RIP mode_config->rotation_property
39abe24 drm/i915: Use the per-plane rotation property
ea0456a drm/msm/mdp5: Advertize 180 degree rotation
88c2533 drm/msm/mdp5: Use per-plane rotation property
6f405b9 drm/msm/mdp5: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0
cf189a7 drm/omap: Use per-plane rotation property
821a368 drm/omap: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0
e8df2c3 drm/atmel-hlcdc: Use per-plane rotation property
f0ffcf6 drm/arm: Use per-plane rotation property
c8ebcd0 drm: Add support for optional per-plane rotation property
b581802 drm/atomic: Reject attempts to use multiple rotation angles at once
7a01eb6 drm: Add drm_rotation_90_or_270()
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* ✗ Ro.CI.BAT: failure for drm: Per-plane rotation etc. (rev2)
2016-07-22 13:43 [PATCH 00/15] drm: Per-plane rotation etc ville.syrjala
` (15 preceding siblings ...)
2016-07-22 14:24 ` ✗ Ro.CI.BAT: failure for drm: Per-plane rotation etc Patchwork
@ 2016-07-22 16:31 ` Patchwork
16 siblings, 0 replies; 41+ messages in thread
From: Patchwork @ 2016-07-22 16:31 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm: Per-plane rotation etc. (rev2)
URL : https://patchwork.freedesktop.org/series/10189/
State : failure
== Summary ==
Series 10189v2 drm: Per-plane rotation etc.
http://patchwork.freedesktop.org/api/1.0/series/10189/revisions/2/mbox
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (fi-skl-i5-6260u)
incomplete -> PASS (fi-skl-i7-6700k)
Test gem_sync:
Subgroup basic-store-each:
pass -> DMESG-FAIL (ro-bdw-i7-5600u)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass -> FAIL (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass -> INCOMPLETE (fi-hsw-i7-4770k)
fi-hsw-i7-4770k total:196 pass:177 dwarn:0 dfail:0 fail:0 skip:18
fi-kbl-qkkr total:244 pass:179 dwarn:28 dfail:1 fail:8 skip:28
fi-skl-i5-6260u total:244 pass:223 dwarn:1 dfail:0 fail:8 skip:12
fi-skl-i7-6700k total:244 pass:210 dwarn:0 dfail:0 fail:8 skip:26
fi-snb-i7-2600 total:244 pass:196 dwarn:0 dfail:0 fail:8 skip:40
ro-bdw-i5-5250u total:244 pass:219 dwarn:4 dfail:0 fail:8 skip:13
ro-bdw-i7-5557U total:244 pass:220 dwarn:3 dfail:0 fail:8 skip:13
ro-bdw-i7-5600u total:244 pass:202 dwarn:0 dfail:1 fail:9 skip:32
ro-bsw-n3050 total:218 pass:173 dwarn:0 dfail:0 fail:2 skip:42
ro-byt-n2820 total:244 pass:197 dwarn:0 dfail:0 fail:9 skip:38
ro-hsw-i3-4010u total:244 pass:212 dwarn:0 dfail:0 fail:8 skip:24
ro-hsw-i7-4770r total:244 pass:212 dwarn:0 dfail:0 fail:8 skip:24
ro-ilk-i7-620lm total:244 pass:172 dwarn:0 dfail:0 fail:9 skip:63
ro-ilk1-i5-650 total:239 pass:172 dwarn:0 dfail:0 fail:9 skip:58
ro-ivb-i7-3770 total:244 pass:203 dwarn:0 dfail:0 fail:8 skip:33
ro-snb-i7-2620M total:244 pass:193 dwarn:0 dfail:0 fail:9 skip:42
ro-skl3-i5-6260u failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_1581/
5ba283d drm-intel-nightly: 2016y-07m-22d-15h-23m-49s UTC integration manifest
ae3dea7 drm/i915: Add horizontal mirroring support for CHV pipe B planes
09d344a drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup
32d9299 drm/i915: Use & instead if == to check for rotations
7cc724e drm: RIP mode_config->rotation_property
db83653 drm/i915: Use the per-plane rotation property
09c5143 drm/msm/mdp5: Advertize 180 degree rotation
5d40902 drm/msm/mdp5: Use per-plane rotation property
4ebc503 drm/msm/mdp5: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0
f364568 drm/omap: Use per-plane rotation property
95b09b0 drm/omap: Set rotation property initial value to BIT(DRM_ROTATE_0) insted of 0
688db65 drm/atmel-hlcdc: Use per-plane rotation property
c380b867 drm/arm: Use per-plane rotation property
94c89f9 drm: Add support for optional per-plane rotation property
535e31d drm/atomic: Reject attempts to use multiple rotation angles at once
6e45c54 drm: Add drm_rotation_90_or_270()
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread