* [PATCH 1/6] drm: Add crtc->name and use it in debug messages
2015-11-12 16:52 [PATCH 0/6] drm: Give crtcs and planes actual names ville.syrjala
@ 2015-11-12 16:52 ` ville.syrjala
2015-11-12 16:52 ` [PATCH 2/6] drm: Add plane->name and use it in debug prints ville.syrjala
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: ville.syrjala @ 2015-11-12 16:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 41 ++++++++++++++-------------
drivers/gpu/drm/drm_atomic_helper.c | 56 +++++++++++++++++++------------------
drivers/gpu/drm/drm_crtc.c | 8 ++++--
drivers/gpu/drm/drm_crtc_helper.c | 24 ++++++++--------
include/drm/drm_crtc.h | 2 ++
5 files changed, 71 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7bb3845..2944655 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -288,8 +288,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
state->crtcs[index] = crtc;
crtc_state->state = state;
- DRM_DEBUG_ATOMIC("Added [CRTC:%d] %p state to %p\n",
- crtc->base.id, crtc_state, state);
+ DRM_DEBUG_ATOMIC("Added [CRTC:%d:%s] %p state to %p\n",
+ crtc->base.id, crtc->name, crtc_state, state);
return crtc_state;
}
@@ -480,8 +480,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
*/
if (state->active && !state->enable) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] active without enabled\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active without enabled\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -490,15 +490,15 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
* be able to trigger. */
if (drm_core_check_feature(crtc->dev, DRIVER_ATOMIC) &&
WARN_ON(state->enable && !state->mode_blob)) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] enabled without mode blob\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled without mode blob\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
if (drm_core_check_feature(crtc->dev, DRIVER_ATOMIC) &&
WARN_ON(!state->enable && state->mode_blob)) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] disabled with mode blob\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] disabled with mode blob\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -980,8 +980,8 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,
}
if (crtc)
- DRM_DEBUG_ATOMIC("Link plane state %p to [CRTC:%d]\n",
- plane_state, crtc->base.id);
+ DRM_DEBUG_ATOMIC("Link plane state %p to [CRTC:%d:%s]\n",
+ plane_state, crtc->base.id, crtc->name);
else
DRM_DEBUG_ATOMIC("Link plane state %p to [NOCRTC]\n",
plane_state);
@@ -1048,8 +1048,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
conn_state->crtc = crtc;
if (crtc)
- DRM_DEBUG_ATOMIC("Link connector state %p to [CRTC:%d]\n",
- conn_state, crtc->base.id);
+ DRM_DEBUG_ATOMIC("Link connector state %p to [CRTC:%d:%s]\n",
+ conn_state, crtc->base.id, crtc->name);
else
DRM_DEBUG_ATOMIC("Link connector state %p to [NOCRTC]\n",
conn_state);
@@ -1088,8 +1088,8 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state,
if (ret)
return ret;
- DRM_DEBUG_ATOMIC("Adding all current connectors for [CRTC:%d] to %p\n",
- crtc->base.id, state);
+ DRM_DEBUG_ATOMIC("Adding all current connectors for [CRTC:%d:%s] to %p\n",
+ crtc->base.id, crtc->name, state);
/*
* Changed connectors are already in @state, so only need to look at the
@@ -1169,8 +1169,9 @@ drm_atomic_connectors_for_crtc(struct drm_atomic_state *state,
num_connected_connectors++;
}
- DRM_DEBUG_ATOMIC("State %p has %i connectors for [CRTC:%d]\n",
- state, num_connected_connectors, crtc->base.id);
+ DRM_DEBUG_ATOMIC("State %p has %i connectors for [CRTC:%d:%s]\n",
+ state, num_connected_connectors,
+ crtc->base.id, crtc->name);
return num_connected_connectors;
}
@@ -1237,8 +1238,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
for_each_crtc_in_state(state, crtc, crtc_state, i) {
ret = drm_atomic_crtc_check(crtc, crtc_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] atomic core check failed\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check failed\n",
+ crtc->base.id, crtc->name);
return ret;
}
}
@@ -1249,8 +1250,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
if (!state->allow_modeset) {
for_each_crtc_in_state(state, crtc, crtc_state, i) {
if (drm_atomic_crtc_needs_modeset(crtc_state)) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] requires full modeset\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full modeset\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
}
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 0c6f621..fc90af50 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -116,9 +116,9 @@ steal_encoder(struct drm_atomic_state *state,
*/
WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d], stealing it\n",
+ DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing it\n",
encoder->base.id, encoder->name,
- encoder_crtc->base.id);
+ encoder_crtc->base.id, encoder_crtc->name);
crtc_state = drm_atomic_get_crtc_state(state, encoder_crtc);
if (IS_ERR(crtc_state))
@@ -211,12 +211,13 @@ update_connector_routing(struct drm_atomic_state *state, int conn_idx)
}
if (new_encoder == connector_state->best_encoder) {
- DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] keeps [ENCODER:%d:%s], now on [CRTC:%d]\n",
+ DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] keeps [ENCODER:%d:%s], now on [CRTC:%d:%s]\n",
connector->base.id,
connector->name,
new_encoder->base.id,
new_encoder->name,
- connector_state->crtc->base.id);
+ connector_state->crtc->base.id,
+ connector_state->crtc->name);
return 0;
}
@@ -243,12 +244,13 @@ update_connector_routing(struct drm_atomic_state *state, int conn_idx)
crtc_state = state->crtc_states[idx];
crtc_state->connectors_changed = true;
- DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d]\n",
+ DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n",
connector->base.id,
connector->name,
new_encoder->base.id,
new_encoder->name,
- connector_state->crtc->base.id);
+ connector_state->crtc->base.id,
+ connector_state->crtc->name);
return 0;
}
@@ -332,8 +334,8 @@ mode_fixup(struct drm_atomic_state *state)
ret = funcs->mode_fixup(crtc, &crtc_state->mode,
&crtc_state->adjusted_mode);
if (!ret) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] fixup failed\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
}
@@ -380,14 +382,14 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
for_each_crtc_in_state(state, crtc, crtc_state, i) {
if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode)) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] mode changed\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode changed\n",
+ crtc->base.id, crtc->name);
crtc_state->mode_changed = true;
}
if (crtc->state->enable != crtc_state->enable) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] enable changed\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enable changed\n",
+ crtc->base.id, crtc->name);
/*
* For clarity this assignment is done here, but
@@ -428,18 +430,18 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
* a full modeset because update_connector_routing force that.
*/
if (crtc->state->active != crtc_state->active) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] active changed\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active changed\n",
+ crtc->base.id, crtc->name);
crtc_state->active_changed = true;
}
if (!drm_atomic_crtc_needs_modeset(crtc_state))
continue;
- DRM_DEBUG_ATOMIC("[CRTC:%d] needs all connectors, enable: %c, active: %c\n",
- crtc->base.id,
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] needs all connectors, enable: %c, active: %c\n",
+ crtc->base.id, crtc->name,
crtc_state->enable ? 'y' : 'n',
- crtc_state->active ? 'y' : 'n');
+ crtc_state->active ? 'y' : 'n');
ret = drm_atomic_add_affected_connectors(state, crtc);
if (ret != 0)
@@ -453,8 +455,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
crtc);
if (crtc_state->enable != !!num_connectors) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] enabled/connectors mismatch\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -517,8 +519,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
ret = funcs->atomic_check(crtc, state->crtc_states[i]);
if (ret) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] atomic driver check failed\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n",
+ crtc->base.id, crtc->name);
return ret;
}
}
@@ -631,8 +633,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
funcs = crtc->helper_private;
- DRM_DEBUG_ATOMIC("disabling [CRTC:%d]\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("disabling [CRTC:%d:%s]\n",
+ crtc->base.id, crtc->name);
/* Right function depends upon target state. */
@@ -743,8 +745,8 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
funcs = crtc->helper_private;
if (crtc->state->enable && funcs->mode_set_nofb) {
- DRM_DEBUG_ATOMIC("modeset on [CRTC:%d]\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("modeset on [CRTC:%d:%s]\n",
+ crtc->base.id, crtc->name);
funcs->mode_set_nofb(crtc);
}
@@ -843,8 +845,8 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
funcs = crtc->helper_private;
if (crtc->state->enable) {
- DRM_DEBUG_ATOMIC("enabling [CRTC:%d]\n",
- crtc->base.id);
+ DRM_DEBUG_ATOMIC("enabling [CRTC:%d:%s]\n",
+ crtc->base.id, crtc->name);
if (funcs->enable)
funcs->enable(crtc);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 24c5434..ea00a69 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -677,6 +677,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
crtc->dev = dev;
crtc->funcs = funcs;
+ if (!crtc->name)
+ crtc->name = "";
+
drm_modeset_lock_init(&crtc->mutex);
ret = drm_mode_object_get(dev, &crtc->base, DRM_MODE_OBJECT_CRTC);
if (ret)
@@ -1801,7 +1804,8 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
copied = 0;
crtc_id = (uint32_t __user *)(unsigned long)card_res->crtc_id_ptr;
drm_for_each_crtc(crtc, dev) {
- DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
+ DRM_DEBUG_KMS("[CRTC:%d:%s]\n",
+ crtc->base.id, crtc->name);
if (put_user(crtc->base.id, crtc_id + copied)) {
ret = -EFAULT;
goto out;
@@ -2646,7 +2650,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
ret = -ENOENT;
goto out;
}
- DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
+ DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
if (crtc_req->mode_valid) {
/* If we have a mode we need a framebuffer. */
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index ef53475..302ee32 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -329,7 +329,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
DRM_DEBUG_KMS("CRTC fixup failed\n");
goto done;
}
- DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
+ DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
crtc->hwmode = *adjusted_mode;
@@ -484,11 +484,13 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
set->fb = NULL;
if (set->fb) {
- DRM_DEBUG_KMS("[CRTC:%d] [FB:%d] #connectors=%d (x y) (%i %i)\n",
- set->crtc->base.id, set->fb->base.id,
- (int)set->num_connectors, set->x, set->y);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] [FB:%d] #connectors=%d (x y) (%i %i)\n",
+ set->crtc->base.id, set->crtc->name,
+ set->fb->base.id,
+ (int)set->num_connectors, set->x, set->y);
} else {
- DRM_DEBUG_KMS("[CRTC:%d] [NOFB]\n", set->crtc->base.id);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] [NOFB]\n",
+ set->crtc->base.id, set->crtc->name);
drm_crtc_helper_disable(set->crtc);
return 0;
}
@@ -628,12 +630,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
connector->encoder->crtc = new_crtc;
}
if (new_crtc) {
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d]\n",
- connector->base.id, connector->name,
- new_crtc->base.id);
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d:%s]\n",
+ connector->base.id, connector->name,
+ new_crtc->base.id, new_crtc->name);
} else {
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [NOCRTC]\n",
- connector->base.id, connector->name);
+ connector->base.id, connector->name);
}
}
@@ -650,8 +652,8 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
if (!drm_crtc_helper_set_mode(set->crtc, set->mode,
set->x, set->y,
save_set.fb)) {
- DRM_ERROR("failed to set mode on [CRTC:%d]\n",
- set->crtc->base.id);
+ DRM_ERROR("failed to set mode on [CRTC:%d:%s]\n",
+ set->crtc->base.id, set->crtc->name);
set->crtc->primary->fb = save_set.fb;
ret = -EINVAL;
goto fail;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3f0c690..a8279b4 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -426,6 +426,8 @@ struct drm_crtc {
struct device_node *port;
struct list_head head;
+ char *name;
+
/*
* crtc mutex
*
--
2.4.10
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/6] drm: Add plane->name and use it in debug prints
2015-11-12 16:52 [PATCH 0/6] drm: Give crtcs and planes actual names ville.syrjala
2015-11-12 16:52 ` [PATCH 1/6] drm: Add crtc->name and use it in debug messages ville.syrjala
@ 2015-11-12 16:52 ` ville.syrjala
2015-11-12 16:52 ` [PATCH 3/6] drm/i915: Use crtc->name in debug messages ville.syrjala
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: ville.syrjala @ 2015-11-12 16:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 12 ++++++------
drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
drivers/gpu/drm/drm_crtc.c | 3 +++
include/drm/drm_crtc.h | 2 ++
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 2944655..df84060 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -543,8 +543,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
state->planes[index] = plane;
plane_state->state = state;
- DRM_DEBUG_ATOMIC("Added [PLANE:%d] %p state to %p\n",
- plane->base.id, plane_state, state);
+ DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n",
+ plane->base.id, plane->name, plane_state, state);
if (plane_state->crtc) {
struct drm_crtc_state *crtc_state;
@@ -755,8 +755,8 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
}
if (plane_switching_crtc(state->state, plane, state)) {
- DRM_DEBUG_ATOMIC("[PLANE:%d] switching CRTC directly\n",
- plane->base.id);
+ DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
+ plane->base.id, plane->name);
return -EINVAL;
}
@@ -1229,8 +1229,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
for_each_plane_in_state(state, plane, plane_state, i) {
ret = drm_atomic_plane_check(plane, plane_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[PLANE:%d] atomic core check failed\n",
- plane->base.id);
+ DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check failed\n",
+ plane->base.id, plane->name);
return ret;
}
}
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index fc90af50..387d95c 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -503,8 +503,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
ret = funcs->atomic_check(plane, plane_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[PLANE:%d] atomic driver check failed\n",
- plane->base.id);
+ DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n",
+ plane->base.id, plane->name);
return ret;
}
}
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index ea00a69..8dc4052 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1174,6 +1174,9 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
drm_modeset_lock_init(&plane->mutex);
+ if (!plane->name)
+ plane->name = "";
+
plane->base.properties = &plane->properties;
plane->dev = dev;
plane->funcs = funcs;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index a8279b4..a08e256 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -848,6 +848,8 @@ struct drm_plane {
struct drm_device *dev;
struct list_head head;
+ char *name;
+
struct drm_modeset_lock mutex;
struct drm_mode_object base;
--
2.4.10
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/6] drm/i915: Use crtc->name in debug messages
2015-11-12 16:52 [PATCH 0/6] drm: Give crtcs and planes actual names ville.syrjala
2015-11-12 16:52 ` [PATCH 1/6] drm: Add crtc->name and use it in debug messages ville.syrjala
2015-11-12 16:52 ` [PATCH 2/6] drm: Add plane->name and use it in debug prints ville.syrjala
@ 2015-11-12 16:52 ` ville.syrjala
2015-11-12 16:52 ` [PATCH 4/6] drm/i915: Use plane->name in debug prints ville.syrjala
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: ville.syrjala @ 2015-11-12 16:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++----------------
drivers/gpu/drm/i915/intel_fbdev.c | 5 ++--
2 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b5f7493..9845687 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4220,8 +4220,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
i = (enum intel_dpll_id) crtc->pipe;
pll = &dev_priv->shared_dplls[i];
- DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n",
- crtc->base.base.id, pll->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n",
+ crtc->base.base.id, crtc->base.name,
+ pll->name);
WARN_ON(shared_dpll[i].crtc_mask);
@@ -4241,8 +4242,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
/* 1:1 mapping between ports and PLLs */
i = (enum intel_dpll_id)intel_dig_port->port;
pll = &dev_priv->shared_dplls[i];
- DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n",
- crtc->base.base.id, pll->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n",
+ crtc->base.base.id, crtc->base.name,
+ pll->name);
WARN_ON(shared_dpll[i].crtc_mask);
goto found;
@@ -4258,9 +4260,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
if (memcmp(&crtc_state->dpll_hw_state,
&shared_dpll[i].hw_state,
sizeof(crtc_state->dpll_hw_state)) == 0) {
- DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, ative %d)\n",
- crtc->base.base.id, pll->name,
- shared_dpll[i].crtc_mask,
+ DRM_DEBUG_KMS("[CRTC:%d:%s] sharing existing %s (crtc mask 0x%08x, ative %d)\n",
+ crtc->base.base.id, crtc->base.name,
+ pll->name, shared_dpll[i].crtc_mask,
pll->active);
goto found;
}
@@ -4270,8 +4272,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
for (i = 0; i < dev_priv->num_shared_dpll; i++) {
pll = &dev_priv->shared_dplls[i];
if (shared_dpll[i].crtc_mask == 0) {
- DRM_DEBUG_KMS("CRTC:%d allocated %s\n",
- crtc->base.base.id, pll->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] allocated %s\n",
+ crtc->base.base.id, crtc->base.name,
+ pll->name);
goto found;
}
}
@@ -4398,8 +4401,9 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
const struct drm_display_mode *adjusted_mode = &state->base.adjusted_mode;
- DRM_DEBUG_KMS("Updating scaler for [CRTC:%i] scaler_user index %u.%u\n",
- intel_crtc->base.base.id, intel_crtc->pipe, SKL_CRTC_INDEX);
+ DRM_DEBUG_KMS("Updating scaler for [CRTC:%d:%s] scaler_user index %u.%u\n",
+ intel_crtc->base.base.id, intel_crtc->base.name,
+ intel_crtc->pipe, SKL_CRTC_INDEX);
return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
&state->scaler_state.scaler_id, DRM_ROTATE_0,
@@ -11643,13 +11647,13 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_plane_state *old_plane_state =
to_intel_plane_state(plane->state);
- int idx = intel_crtc->base.base.id, ret;
int i = drm_plane_index(plane);
bool mode_changed = needs_modeset(crtc_state);
bool was_crtc_enabled = crtc->state->active;
bool is_crtc_enabled = crtc_state->active;
bool turn_off, turn_on, visible, was_visible;
struct drm_framebuffer *fb = plane_state->fb;
+ int ret;
if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
plane->type != DRM_PLANE_TYPE_CURSOR) {
@@ -11675,7 +11679,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
turn_off = was_visible && (!visible || mode_changed);
turn_on = visible && (!was_visible || mode_changed);
- DRM_DEBUG_ATOMIC("[CRTC:%i] has [PLANE:%i] with fb %i\n", idx,
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%i] with fb %i\n",
+ intel_crtc->base.base.id,
+ intel_crtc->base.name,
plane->base.id, fb ? fb->base.id : -1);
DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n",
@@ -11981,7 +11987,8 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
struct intel_plane_state *state;
struct drm_framebuffer *fb;
- DRM_DEBUG_KMS("[CRTC:%d]%s config %p for pipe %c\n", crtc->base.base.id,
+ DRM_DEBUG_KMS("[CRTC:%d:%s]%s config %p for pipe %c\n",
+ crtc->base.base.id, crtc->base.name,
context, pipe_config, pipe_name(crtc->pipe));
DRM_DEBUG_KMS("cpu_transcoder: %c\n", transcoder_name(pipe_config->cpu_transcoder));
@@ -12753,8 +12760,8 @@ check_crtc_state(struct drm_device *dev, struct drm_atomic_state *old_state)
pipe_config->base.crtc = crtc;
pipe_config->base.state = old_state;
- DRM_DEBUG_KMS("[CRTC:%d]\n",
- crtc->base.id);
+ DRM_DEBUG_KMS("[CRTC:%d:%s]\n",
+ crtc->base.id, crtc->name);
active = dev_priv->display.get_pipe_config(intel_crtc,
pipe_config);
@@ -13390,8 +13397,8 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
state = drm_atomic_state_alloc(dev);
if (!state) {
- DRM_DEBUG_KMS("[CRTC:%d] crtc restore failed, out of memory",
- crtc->base.id);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] crtc restore failed, out of memory",
+ crtc->base.id, crtc->name);
return;
}
@@ -15193,8 +15200,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
if (INTEL_INFO(dev)->gen < 4 && !intel_check_plane_mapping(crtc)) {
bool plane;
- DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
- crtc->base.base.id);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] wrong plane connection detected!\n",
+ crtc->base.base.id, crtc->base.name);
/* Pipe has the wrong plane attached and the plane is active.
* Temporarily change the plane mapping and disable everything
@@ -15227,8 +15234,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
* functions or because of calls to intel_crtc_disable_noatomic,
* or because the pipe is force-enabled due to the
* pipe A quirk. */
- DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was %s, now %s\n",
- crtc->base.base.id,
+ DRM_DEBUG_KMS("[CRTC:%d:%s] hw state adjusted, was %s, now %s\n",
+ crtc->base.base.id, crtc->base.name,
crtc->base.state->enable ? "enabled" : "disabled",
crtc->active ? "enabled" : "disabled");
@@ -15390,8 +15397,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
readout_plane_state(crtc);
- DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
- crtc->base.base.id,
+ DRM_DEBUG_KMS("[CRTC:%d:%s] hw state readout: %s\n",
+ crtc->base.base.id, crtc->base.name,
crtc->active ? "enabled" : "disabled");
}
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 98772d3..7670440 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -472,10 +472,9 @@ retry:
}
crtcs[i] = new_crtc;
- DRM_DEBUG_KMS("connector %s on pipe %c [CRTC:%d]: %dx%d%s\n",
+ DRM_DEBUG_KMS("connector %s on [CRTC:%d:%s]: %dx%d%s\n",
connector->name,
- pipe_name(to_intel_crtc(encoder->crtc)->pipe),
- encoder->crtc->base.id,
+ encoder->crtc->base.id, encoder->crtc->name,
modes[i]->hdisplay, modes[i]->vdisplay,
modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" :"");
--
2.4.10
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/6] drm/i915: Use plane->name in debug prints
2015-11-12 16:52 [PATCH 0/6] drm: Give crtcs and planes actual names ville.syrjala
` (2 preceding siblings ...)
2015-11-12 16:52 ` [PATCH 3/6] drm/i915: Use crtc->name in debug messages ville.syrjala
@ 2015-11-12 16:52 ` ville.syrjala
2015-11-12 16:52 ` [PATCH 5/6] drm/i915: Set crtc->name to "pipe A", "pipe B", etc ville.syrjala
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: ville.syrjala @ 2015-11-12 16:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9845687..b628dab 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4433,9 +4433,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
bool force_detach = !fb || !plane_state->visible;
- DRM_DEBUG_KMS("Updating scaler for [PLANE:%d] scaler_user index %u.%u\n",
- intel_plane->base.base.id, intel_crtc->pipe,
- drm_plane_index(&intel_plane->base));
+ DRM_DEBUG_KMS("Updating scaler for [PLANE:%d:%s] scaler_user index %u.%u\n",
+ intel_plane->base.base.id, intel_plane->base.name,
+ intel_crtc->pipe, drm_plane_index(&intel_plane->base));
ret = skl_update_scaler(crtc_state, force_detach,
drm_plane_index(&intel_plane->base),
@@ -4451,8 +4451,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
/* check colorkey */
if (plane_state->ckey.flags != I915_SET_COLORKEY_NONE) {
- DRM_DEBUG_KMS("[PLANE:%d] scaling with color key not allowed",
- intel_plane->base.base.id);
+ DRM_DEBUG_KMS("[PLANE:%d:%s] scaling with color key not allowed",
+ intel_plane->base.base.id,
+ intel_plane->base.name);
return -EINVAL;
}
@@ -4471,8 +4472,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
case DRM_FORMAT_VYUY:
break;
default:
- DRM_DEBUG_KMS("[PLANE:%d] FB:%d unsupported scaling format 0x%x\n",
- intel_plane->base.base.id, fb->base.id, fb->pixel_format);
+ DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n",
+ intel_plane->base.base.id, intel_plane->base.name,
+ fb->base.id, fb->pixel_format);
return -EINVAL;
}
@@ -11679,13 +11681,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
turn_off = was_visible && (!visible || mode_changed);
turn_on = visible && (!was_visible || mode_changed);
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%i] with fb %i\n",
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n",
intel_crtc->base.base.id,
intel_crtc->base.name,
- plane->base.id, fb ? fb->base.id : -1);
+ plane->base.id, plane->name,
+ fb ? fb->base.id : -1);
- DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n",
- plane->base.id, was_visible, visible,
+ DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n",
+ plane->base.id, plane->name,
+ was_visible, visible,
turn_off, turn_on, mode_changed);
if (turn_on) {
@@ -12088,18 +12092,20 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
state = to_intel_plane_state(plane->state);
fb = state->base.fb;
if (!fb) {
- DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d "
- "disabled, scaler_id = %d\n",
+ DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d "
+ "disabled, scaler_id = %d\n",
plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD",
- plane->base.id, intel_plane->pipe,
+ plane->base.id, plane->name,
+ intel_plane->pipe,
(crtc->base.primary == plane) ? 0 : intel_plane->plane + 1,
drm_plane_index(plane), state->scaler_id);
continue;
}
- DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d enabled",
+ DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d enabled",
plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD",
- plane->base.id, intel_plane->pipe,
+ plane->base.id, plane->name,
+ intel_plane->pipe,
crtc->base.primary == plane ? 0 : intel_plane->plane + 1,
drm_plane_index(plane));
DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = 0x%x",
--
2.4.10
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/6] drm/i915: Set crtc->name to "pipe A", "pipe B", etc.
2015-11-12 16:52 [PATCH 0/6] drm: Give crtcs and planes actual names ville.syrjala
` (3 preceding siblings ...)
2015-11-12 16:52 ` [PATCH 4/6] drm/i915: Use plane->name in debug prints ville.syrjala
@ 2015-11-12 16:52 ` ville.syrjala
2015-11-12 16:52 ` [PATCH 6/6] drm/i915: Give meaningful names to all the planes ville.syrjala
2015-11-17 10:11 ` [PATCH 0/6] drm: Give crtcs and planes actual names Daniel Vetter
6 siblings, 0 replies; 12+ messages in thread
From: ville.syrjala @ 2015-11-12 16:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b628dab..2b5e81a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10711,6 +10711,7 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct intel_unpin_work *work;
+ char *name;
spin_lock_irq(&dev->event_lock);
work = intel_crtc->unpin_work;
@@ -10722,8 +10723,13 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
kfree(work);
}
+ /*
+ * drm_crtc_cleanup() zeroes the structure, so
+ * need an extra dance to avoid leaking the name.
+ */
+ name = crtc->name;
drm_crtc_cleanup(crtc);
-
+ kfree(name);
kfree(intel_crtc);
}
@@ -14036,6 +14042,11 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
if (intel_crtc == NULL)
return;
+ intel_crtc->base.name = kasprintf(GFP_KERNEL, "pipe %c",
+ pipe_name(pipe));
+ if (!intel_crtc->base.name)
+ return;
+
crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
if (!crtc_state)
goto fail;
@@ -14106,6 +14117,7 @@ fail:
if (cursor)
drm_plane_cleanup(cursor);
kfree(crtc_state);
+ kfree(intel_crtc->base.name);
kfree(intel_crtc);
}
--
2.4.10
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/6] drm/i915: Give meaningful names to all the planes
2015-11-12 16:52 [PATCH 0/6] drm: Give crtcs and planes actual names ville.syrjala
` (4 preceding siblings ...)
2015-11-12 16:52 ` [PATCH 5/6] drm/i915: Set crtc->name to "pipe A", "pipe B", etc ville.syrjala
@ 2015-11-12 16:52 ` ville.syrjala
2015-11-12 17:38 ` Emil Velikov
2015-11-17 10:11 ` [PATCH 0/6] drm: Give crtcs and planes actual names Daniel Vetter
6 siblings, 1 reply; 12+ messages in thread
From: ville.syrjala @ 2015-11-12 16:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Let's name our planes in a way that makes sense wrt. the spec:
- skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc.
- g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc.
- pre-g4x -> "plane A", "cursor B" etc.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_sprite.c | 14 ++++++++++++++
2 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2b5e81a..82b2f58 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13788,7 +13788,15 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
void intel_plane_destroy(struct drm_plane *plane)
{
struct intel_plane *intel_plane = to_intel_plane(plane);
+ char *name;
+
+ /*
+ * drm_plane_cleanup() zeroes the structure, so
+ * need an extra dance to avoid leaking the name.
+ */
+ name = plane->name;
drm_plane_cleanup(plane);
+ kfree(name);
kfree(intel_plane);
}
@@ -13838,6 +13846,21 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
primary->plane = !pipe;
+ if (INTEL_INFO(dev)->gen >= 9)
+ primary->base.name = kasprintf(GFP_KERNEL, "plane 1%c",
+ pipe_name(pipe));
+ else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
+ primary->base.name = kasprintf(GFP_KERNEL, "primary %c",
+ pipe_name(pipe));
+ else
+ primary->base.name = kasprintf(GFP_KERNEL, "plane %c",
+ plane_name(primary->plane));
+ if (!primary->base.name) {
+ kfree(state);
+ kfree(primary);
+ return NULL;
+ }
+
if (INTEL_INFO(dev)->gen >= 9) {
intel_primary_formats = skl_primary_formats;
num_formats = ARRAY_SIZE(skl_primary_formats);
@@ -13987,6 +14010,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
cursor->commit_plane = intel_commit_cursor_plane;
cursor->disable_plane = intel_disable_cursor_plane;
+ cursor->base.name = kasprintf(GFP_KERNEL, "cursor %c",
+ pipe_name(pipe));
+ if (!cursor->base.name) {
+ kfree(state);
+ kfree(cursor);
+ return NULL;
+ }
+
drm_universal_plane_init(dev, &cursor->base, 0,
&intel_plane_funcs,
intel_cursor_formats,
@@ -14113,9 +14144,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
fail:
if (primary)
- drm_plane_cleanup(primary);
+ intel_plane_destroy(primary);
if (cursor)
- drm_plane_cleanup(cursor);
+ intel_plane_destroy(cursor);
kfree(crtc_state);
kfree(intel_crtc->base.name);
kfree(intel_crtc);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a2c15f8..b1520f1 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1119,7 +1119,21 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
intel_plane->check_plane = intel_check_sprite_plane;
intel_plane->commit_plane = intel_commit_sprite_plane;
+
+ if (INTEL_INFO(dev)->gen >= 9)
+ intel_plane->base.name = kasprintf(GFP_KERNEL, "plane %d%c",
+ plane + 2, pipe_name(pipe));
+ else
+ intel_plane->base.name = kasprintf(GFP_KERNEL, "sprite %c",
+ sprite_name(pipe, plane));
+ if (!intel_plane->base.name) {
+ kfree(state);
+ kfree(intel_plane);
+ return -ENOMEM;
+ }
+
possible_crtcs = (1 << pipe);
+
ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
&intel_plane_funcs,
plane_formats, num_plane_formats,
--
2.4.10
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 6/6] drm/i915: Give meaningful names to all the planes
2015-11-12 16:52 ` [PATCH 6/6] drm/i915: Give meaningful names to all the planes ville.syrjala
@ 2015-11-12 17:38 ` Emil Velikov
2015-11-12 17:49 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Emil Velikov @ 2015-11-12 17:38 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org, ML dri-devel
Hi Ville,
On 12 November 2015 at 16:52, <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's name our planes in a way that makes sense wrt. the spec:
> - skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc.
> - g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc.
> - pre-g4x -> "plane A", "cursor B" etc.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_sprite.c | 14 ++++++++++++++
> 2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2b5e81a..82b2f58 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13788,7 +13788,15 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
> void intel_plane_destroy(struct drm_plane *plane)
> {
> struct intel_plane *intel_plane = to_intel_plane(plane);
> + char *name;
> +
> + /*
> + * drm_plane_cleanup() zeroes the structure, so
> + * need an extra dance to avoid leaking the name.
> + */
> + name = plane->name;
> drm_plane_cleanup(plane);
> + kfree(name);
> kfree(intel_plane);
> }
>
> @@ -13838,6 +13846,21 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
> primary->plane = !pipe;
>
> + if (INTEL_INFO(dev)->gen >= 9)
> + primary->base.name = kasprintf(GFP_KERNEL, "plane 1%c",
> + pipe_name(pipe));
> + else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> + primary->base.name = kasprintf(GFP_KERNEL, "primary %c",
> + pipe_name(pipe));
> + else
> + primary->base.name = kasprintf(GFP_KERNEL, "plane %c",
> + plane_name(primary->plane));
> + if (!primary->base.name) {
> + kfree(state);
> + kfree(primary);
> + return NULL;
Worth adding a label and doing all the teardown there ? (same goes for
the rest of the patch)
> + }
> +
> if (INTEL_INFO(dev)->gen >= 9) {
> intel_primary_formats = skl_primary_formats;
> num_formats = ARRAY_SIZE(skl_primary_formats);
> @@ -13987,6 +14010,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> cursor->commit_plane = intel_commit_cursor_plane;
> cursor->disable_plane = intel_disable_cursor_plane;
>
> + cursor->base.name = kasprintf(GFP_KERNEL, "cursor %c",
> + pipe_name(pipe));
> + if (!cursor->base.name) {
> + kfree(state);
> + kfree(cursor);
> + return NULL;
> + }
> +
> drm_universal_plane_init(dev, &cursor->base, 0,
> &intel_plane_funcs,
> intel_cursor_formats,
> @@ -14113,9 +14144,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>
> fail:
> if (primary)
> - drm_plane_cleanup(primary);
> + intel_plane_destroy(primary);
> if (cursor)
> - drm_plane_cleanup(cursor);
> + intel_plane_destroy(cursor);
Something feels strange here. We are either leaking memory before or
we'll end up with double free after your patch. Worth
checking/mentioning in the commit message ?
Regards,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 6/6] drm/i915: Give meaningful names to all the planes
2015-11-12 17:38 ` Emil Velikov
@ 2015-11-12 17:49 ` Ville Syrjälä
2015-11-12 18:32 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2015-11-12 17:49 UTC (permalink / raw)
To: Emil Velikov; +Cc: intel-gfx@lists.freedesktop.org, ML dri-devel
On Thu, Nov 12, 2015 at 05:38:48PM +0000, Emil Velikov wrote:
> Hi Ville,
>
> On 12 November 2015 at 16:52, <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Let's name our planes in a way that makes sense wrt. the spec:
> > - skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc.
> > - g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc.
> > - pre-g4x -> "plane A", "cursor B" etc.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++++++++++++++--
> > drivers/gpu/drm/i915/intel_sprite.c | 14 ++++++++++++++
> > 2 files changed, 47 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 2b5e81a..82b2f58 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13788,7 +13788,15 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
> > void intel_plane_destroy(struct drm_plane *plane)
> > {
> > struct intel_plane *intel_plane = to_intel_plane(plane);
> > + char *name;
> > +
> > + /*
> > + * drm_plane_cleanup() zeroes the structure, so
> > + * need an extra dance to avoid leaking the name.
> > + */
> > + name = plane->name;
> > drm_plane_cleanup(plane);
> > + kfree(name);
> > kfree(intel_plane);
> > }
> >
> > @@ -13838,6 +13846,21 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
> > primary->plane = !pipe;
> >
> > + if (INTEL_INFO(dev)->gen >= 9)
> > + primary->base.name = kasprintf(GFP_KERNEL, "plane 1%c",
> > + pipe_name(pipe));
> > + else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> > + primary->base.name = kasprintf(GFP_KERNEL, "primary %c",
> > + pipe_name(pipe));
> > + else
> > + primary->base.name = kasprintf(GFP_KERNEL, "plane %c",
> > + plane_name(primary->plane));
> > + if (!primary->base.name) {
> > + kfree(state);
> > + kfree(primary);
> > + return NULL;
> Worth adding a label and doing all the teardown there ? (same goes for
> the rest of the patch)
Dunno. Was feeling lazy, and so didn't go the extra mile.
>
> > + }
> > +
> > if (INTEL_INFO(dev)->gen >= 9) {
> > intel_primary_formats = skl_primary_formats;
> > num_formats = ARRAY_SIZE(skl_primary_formats);
> > @@ -13987,6 +14010,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> > cursor->commit_plane = intel_commit_cursor_plane;
> > cursor->disable_plane = intel_disable_cursor_plane;
> >
> > + cursor->base.name = kasprintf(GFP_KERNEL, "cursor %c",
> > + pipe_name(pipe));
> > + if (!cursor->base.name) {
> > + kfree(state);
> > + kfree(cursor);
> > + return NULL;
> > + }
> > +
> > drm_universal_plane_init(dev, &cursor->base, 0,
> > &intel_plane_funcs,
> > intel_cursor_formats,
> > @@ -14113,9 +14144,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> >
> > fail:
> > if (primary)
> > - drm_plane_cleanup(primary);
> > + intel_plane_destroy(primary);
> > if (cursor)
> > - drm_plane_cleanup(cursor);
> > + intel_plane_destroy(cursor);
> Something feels strange here. We are either leaking memory before or
> we'll end up with double free after your patch. Worth
> checking/mentioning in the commit message ?
Yeah, I think we were leaking here. Forgot to add a note.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 6/6] drm/i915: Give meaningful names to all the planes
2015-11-12 17:49 ` Ville Syrjälä
@ 2015-11-12 18:32 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2015-11-12 18:32 UTC (permalink / raw)
To: Emil Velikov; +Cc: intel-gfx@lists.freedesktop.org, ML dri-devel
On Thu, Nov 12, 2015 at 07:49:19PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 12, 2015 at 05:38:48PM +0000, Emil Velikov wrote:
> > Hi Ville,
> >
> > On 12 November 2015 at 16:52, <ville.syrjala@linux.intel.com> wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Let's name our planes in a way that makes sense wrt. the spec:
> > > - skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc.
> > > - g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc.
> > > - pre-g4x -> "plane A", "cursor B" etc.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++++++++++++++--
> > > drivers/gpu/drm/i915/intel_sprite.c | 14 ++++++++++++++
> > > 2 files changed, 47 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 2b5e81a..82b2f58 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -13788,7 +13788,15 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
> > > void intel_plane_destroy(struct drm_plane *plane)
> > > {
> > > struct intel_plane *intel_plane = to_intel_plane(plane);
> > > + char *name;
> > > +
> > > + /*
> > > + * drm_plane_cleanup() zeroes the structure, so
> > > + * need an extra dance to avoid leaking the name.
> > > + */
> > > + name = plane->name;
> > > drm_plane_cleanup(plane);
> > > + kfree(name);
> > > kfree(intel_plane);
> > > }
> > >
> > > @@ -13838,6 +13846,21 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> > > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
> > > primary->plane = !pipe;
> > >
> > > + if (INTEL_INFO(dev)->gen >= 9)
> > > + primary->base.name = kasprintf(GFP_KERNEL, "plane 1%c",
> > > + pipe_name(pipe));
> > > + else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> > > + primary->base.name = kasprintf(GFP_KERNEL, "primary %c",
> > > + pipe_name(pipe));
> > > + else
> > > + primary->base.name = kasprintf(GFP_KERNEL, "plane %c",
> > > + plane_name(primary->plane));
> > > + if (!primary->base.name) {
> > > + kfree(state);
> > > + kfree(primary);
> > > + return NULL;
> > Worth adding a label and doing all the teardown there ? (same goes for
> > the rest of the patch)
>
> Dunno. Was feeling lazy, and so didn't go the extra mile.
After a better look I saw that I fumbled the error paths in the crtc
name patch too. So I went on to clean things up a bit. I think I'll
repost the lot since there are now more patches.
>
> >
> > > + }
> > > +
> > > if (INTEL_INFO(dev)->gen >= 9) {
> > > intel_primary_formats = skl_primary_formats;
> > > num_formats = ARRAY_SIZE(skl_primary_formats);
> > > @@ -13987,6 +14010,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> > > cursor->commit_plane = intel_commit_cursor_plane;
> > > cursor->disable_plane = intel_disable_cursor_plane;
> > >
> > > + cursor->base.name = kasprintf(GFP_KERNEL, "cursor %c",
> > > + pipe_name(pipe));
> > > + if (!cursor->base.name) {
> > > + kfree(state);
> > > + kfree(cursor);
> > > + return NULL;
> > > + }
> > > +
> > > drm_universal_plane_init(dev, &cursor->base, 0,
> > > &intel_plane_funcs,
> > > intel_cursor_formats,
> > > @@ -14113,9 +14144,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> > >
> > > fail:
> > > if (primary)
> > > - drm_plane_cleanup(primary);
> > > + intel_plane_destroy(primary);
> > > if (cursor)
> > > - drm_plane_cleanup(cursor);
> > > + intel_plane_destroy(cursor);
> > Something feels strange here. We are either leaking memory before or
> > we'll end up with double free after your patch. Worth
> > checking/mentioning in the commit message ?
>
> Yeah, I think we were leaking here. Forgot to add a note.
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/6] drm: Give crtcs and planes actual names
2015-11-12 16:52 [PATCH 0/6] drm: Give crtcs and planes actual names ville.syrjala
` (5 preceding siblings ...)
2015-11-12 16:52 ` [PATCH 6/6] drm/i915: Give meaningful names to all the planes ville.syrjala
@ 2015-11-17 10:11 ` Daniel Vetter
2015-11-17 10:20 ` [Intel-gfx] " Jani Nikula
6 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2015-11-17 10:11 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, dri-devel
On Thu, Nov 12, 2015 at 06:52:20PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I got sick and tired of staring at the line noise produced by drm.debug=0x1e,
> so I decided to give the crtcs and planes human readable names. Each driver
> can give whatever names it wants, and for i915 I gave something that makes
> some sense w.r.t. to the spec.
>
> The only magic gotcha here is that if the driver dynamically allocates the
> name, it must be careful around drm_{crtc,plane}_cleanup() cause those
> guys just memset the entire structure to 0. I didn't want to put the kfree()
> into the cleanup functions to avoid having to kstrdup("") in the fallback
> case or forcing drivers to always use a dynamic allocation.
I avoiding the kstrdup("") is a bit a hack, especially since we could put
somethinig useful in there like "idx-%i", drm_plane_index(). The index is
used by a bunch of things (both internally and in ioctl structs), so
pretty handy.
-Daniel
>
> Ville Syrjälä (6):
> drm: Add crtc->name and use it in debug messages
> drm: Add plane->name and use it in debug prints
> drm/i915: Use crtc->name in debug messages
> drm/i915: Use plane->name in debug prints
> drm/i915: Set crtc->name to "pipe A", "pipe B", etc.
> drm/i915: Give meaningful names to all the planes
>
> drivers/gpu/drm/drm_atomic.c | 53 ++++++++-------
> drivers/gpu/drm/drm_atomic_helper.c | 60 +++++++++--------
> drivers/gpu/drm/drm_crtc.c | 11 ++-
> drivers/gpu/drm/drm_crtc_helper.c | 24 ++++---
> drivers/gpu/drm/i915/intel_display.c | 127 +++++++++++++++++++++++------------
> drivers/gpu/drm/i915/intel_fbdev.c | 5 +-
> drivers/gpu/drm/i915/intel_sprite.c | 14 ++++
> include/drm/drm_crtc.h | 4 ++
> 8 files changed, 185 insertions(+), 113 deletions(-)
>
> --
> 2.4.10
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Intel-gfx] [PATCH 0/6] drm: Give crtcs and planes actual names
2015-11-17 10:11 ` [PATCH 0/6] drm: Give crtcs and planes actual names Daniel Vetter
@ 2015-11-17 10:20 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2015-11-17 10:20 UTC (permalink / raw)
To: Daniel Vetter, ville.syrjala; +Cc: intel-gfx, dri-devel
On Tue, 17 Nov 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Nov 12, 2015 at 06:52:20PM +0200, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> I got sick and tired of staring at the line noise produced by drm.debug=0x1e,
>> so I decided to give the crtcs and planes human readable names. Each driver
>> can give whatever names it wants, and for i915 I gave something that makes
>> some sense w.r.t. to the spec.
>>
>> The only magic gotcha here is that if the driver dynamically allocates the
>> name, it must be careful around drm_{crtc,plane}_cleanup() cause those
>> guys just memset the entire structure to 0. I didn't want to put the kfree()
>> into the cleanup functions to avoid having to kstrdup("") in the fallback
>> case or forcing drivers to always use a dynamic allocation.
>
> I avoiding the kstrdup("") is a bit a hack, especially since we could put
> somethinig useful in there like "idx-%i", drm_plane_index(). The index is
> used by a bunch of things (both internally and in ioctl structs), so
> pretty handy.
Find the latest version of the series first. ;)
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread