* [PATCH] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths
@ 2016-05-13 15:13 ville.syrjala
2016-05-16 7:47 ` Jani Nikula
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: ville.syrjala @ 2016-05-13 15:13 UTC (permalink / raw)
To: dri-devel
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
DRM_DEBUG_ATOMIC generates a lot of noise that no one normally cares
about. However error paths everyone cares about, so hiding thea error
debugs under DRM_DEBUG_ATOMIC is a bad idea. Let's use DRM_DEBUG_KMS
for those instead.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 60 ++++++++++++++++++-------------------
drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++-------------------
2 files changed, 60 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 86e89db02ed7..1479bfd8744b 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -554,8 +554,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
*/
if (state->active && !state->enable) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active without enabled\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] active without enabled\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -564,15 +564,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:%s] enabled without mode blob\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[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:%s] disabled with mode blob\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] disabled with mode blob\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -587,8 +587,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
* pipe.
*/
if (state->event && !state->active && !crtc->state->active) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] requesting event but off\n",
- crtc->base.id);
+ DRM_DEBUG_KMS("[CRTC:%d] requesting event but off\n",
+ crtc->base.id);
return -EINVAL;
}
@@ -802,10 +802,10 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
/* either *both* CRTC and FB must be set, or neither */
if (WARN_ON(state->crtc && !state->fb)) {
- DRM_DEBUG_ATOMIC("CRTC set but no FB\n");
+ DRM_DEBUG_KMS("CRTC set but no FB\n");
return -EINVAL;
} else if (WARN_ON(state->fb && !state->crtc)) {
- DRM_DEBUG_ATOMIC("FB set but no CRTC\n");
+ DRM_DEBUG_KMS("FB set but no CRTC\n");
return -EINVAL;
}
@@ -815,15 +815,15 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
/* Check whether this plane is usable on this CRTC */
if (!(plane->possible_crtcs & drm_crtc_mask(state->crtc))) {
- DRM_DEBUG_ATOMIC("Invalid crtc for plane\n");
+ DRM_DEBUG_KMS("Invalid crtc for plane\n");
return -EINVAL;
}
/* Check whether this plane supports the fb pixel format. */
ret = drm_plane_check_pixel_format(plane, state->fb->pixel_format);
if (ret) {
- DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
- drm_get_format_name(state->fb->pixel_format));
+ DRM_DEBUG_KMS("Invalid pixel format %s\n",
+ drm_get_format_name(state->fb->pixel_format));
return ret;
}
@@ -832,9 +832,9 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
state->crtc_x > INT_MAX - (int32_t) state->crtc_w ||
state->crtc_h > INT_MAX ||
state->crtc_y > INT_MAX - (int32_t) state->crtc_h) {
- DRM_DEBUG_ATOMIC("Invalid CRTC coordinates %ux%u+%d+%d\n",
- state->crtc_w, state->crtc_h,
- state->crtc_x, state->crtc_y);
+ DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
+ state->crtc_w, state->crtc_h,
+ state->crtc_x, state->crtc_y);
return -ERANGE;
}
@@ -846,18 +846,18 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
state->src_x > fb_width - state->src_w ||
state->src_h > fb_height ||
state->src_y > fb_height - state->src_h) {
- DRM_DEBUG_ATOMIC("Invalid source coordinates "
- "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
- state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
- state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
- state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
- state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10);
+ DRM_DEBUG_KMS("Invalid source coordinates "
+ "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
+ state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
+ state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
+ state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
+ state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10);
return -ENOSPC;
}
if (plane_switching_crtc(state->state, plane, state)) {
- DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
- plane->base.id, plane->name);
+ DRM_DEBUG_KMS("[PLANE:%d:%s] switching CRTC directly\n",
+ plane->base.id, plane->name);
return -EINVAL;
}
@@ -1330,8 +1330,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:%s] atomic core check failed\n",
- plane->base.id, plane->name);
+ DRM_DEBUG_KMS("[PLANE:%d:%s] atomic core check failed\n",
+ plane->base.id, plane->name);
return ret;
}
}
@@ -1339,8 +1339,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:%s] atomic core check failed\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] atomic core check failed\n",
+ crtc->base.id, crtc->name);
return ret;
}
}
@@ -1351,8 +1351,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:%s] requires full modeset\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[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 43a0b3dfa846..18ebb1a08d6c 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -115,9 +115,9 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
if (new_encoder) {
if (encoder_mask & (1 << drm_encoder_index(new_encoder))) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
- new_encoder->base.id, new_encoder->name,
- connector->base.id, connector->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
+ new_encoder->base.id, new_encoder->name,
+ connector->base.id, connector->name);
return -EINVAL;
}
@@ -151,11 +151,11 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
continue;
if (!disable_conflicting_encoders) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
- encoder->base.id, encoder->name,
- connector->state->crtc->base.id,
- connector->state->crtc->name,
- connector->base.id, connector->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
+ encoder->base.id, encoder->name,
+ connector->state->crtc->base.id,
+ connector->state->crtc->name,
+ connector->base.id, connector->name);
return -EINVAL;
}
@@ -302,17 +302,17 @@ update_connector_routing(struct drm_atomic_state *state,
new_encoder = funcs->best_encoder(connector);
if (!new_encoder) {
- DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
- connector->base.id,
- connector->name);
+ DRM_DEBUG_KMS("No suitable encoder found for [CONNECTOR:%d:%s]\n",
+ connector->base.id,
+ connector->name);
return -EINVAL;
}
if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d]\n",
- new_encoder->base.id,
- new_encoder->name,
- connector_state->crtc->base.id);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] incompatible with [CRTC:%d]\n",
+ new_encoder->base.id,
+ new_encoder->name,
+ connector_state->crtc->base.id);
return -EINVAL;
}
@@ -388,7 +388,7 @@ mode_fixup(struct drm_atomic_state *state)
ret = drm_bridge_mode_fixup(encoder->bridge, &crtc_state->mode,
&crtc_state->adjusted_mode);
if (!ret) {
- DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
+ DRM_DEBUG_KMS("Bridge fixup failed\n");
return -EINVAL;
}
@@ -396,16 +396,16 @@ mode_fixup(struct drm_atomic_state *state)
ret = funcs->atomic_check(encoder, crtc_state,
conn_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n",
- encoder->base.id, encoder->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] check failed\n",
+ encoder->base.id, encoder->name);
return ret;
}
} else if (funcs && funcs->mode_fixup) {
ret = funcs->mode_fixup(encoder, &crtc_state->mode,
&crtc_state->adjusted_mode);
if (!ret) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n",
- encoder->base.id, encoder->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] fixup failed\n",
+ encoder->base.id, encoder->name);
return -EINVAL;
}
}
@@ -425,8 +425,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:%s] fixup failed\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] fixup failed\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
}
@@ -549,8 +549,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
return ret;
if (crtc_state->enable != has_connectors) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] enabled/connectors mismatch\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -597,8 +597,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
ret = funcs->atomic_check(plane, plane_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n",
- plane->base.id, plane->name);
+ DRM_DEBUG_KMS("[PLANE:%d:%s] atomic driver check failed\n",
+ plane->base.id, plane->name);
return ret;
}
}
@@ -613,8 +613,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:%s] atomic driver check failed\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] atomic driver check failed\n",
+ crtc->base.id, crtc->name);
return ret;
}
}
@@ -2367,8 +2367,8 @@ retry:
/* Make sure we don't accidentally do a full modeset. */
state->allow_modeset = false;
if (!crtc_state->active) {
- DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
- crtc->base.id);
+ DRM_DEBUG_KMS("[CRTC:%d] disabled, rejecting legacy flip\n",
+ crtc->base.id);
ret = -EINVAL;
goto fail;
}
--
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] 6+ messages in thread
* Re: [PATCH] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths
2016-05-13 15:13 [PATCH] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths ville.syrjala
@ 2016-05-16 7:47 ` Jani Nikula
2016-05-16 12:21 ` Emil Velikov
2017-11-15 18:38 ` [PATCH v2] " Ville Syrjala
2 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2016-05-16 7:47 UTC (permalink / raw)
To: ville.syrjala, dri-devel
On Fri, 13 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> DRM_DEBUG_ATOMIC generates a lot of noise that no one normally cares
> about. However error paths everyone cares about, so hiding thea error
> debugs under DRM_DEBUG_ATOMIC is a bad idea. Let's use DRM_DEBUG_KMS
> for those instead.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Agreed.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 60 ++++++++++++++++++-------------------
> drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++-------------------
> 2 files changed, 60 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 86e89db02ed7..1479bfd8744b 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -554,8 +554,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
> */
>
> if (state->active && !state->enable) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active without enabled\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] active without enabled\n",
> + crtc->base.id, crtc->name);
> return -EINVAL;
> }
>
> @@ -564,15 +564,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:%s] enabled without mode blob\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[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:%s] disabled with mode blob\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] disabled with mode blob\n",
> + crtc->base.id, crtc->name);
> return -EINVAL;
> }
>
> @@ -587,8 +587,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
> * pipe.
> */
> if (state->event && !state->active && !crtc->state->active) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d] requesting event but off\n",
> - crtc->base.id);
> + DRM_DEBUG_KMS("[CRTC:%d] requesting event but off\n",
> + crtc->base.id);
> return -EINVAL;
> }
>
> @@ -802,10 +802,10 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
>
> /* either *both* CRTC and FB must be set, or neither */
> if (WARN_ON(state->crtc && !state->fb)) {
> - DRM_DEBUG_ATOMIC("CRTC set but no FB\n");
> + DRM_DEBUG_KMS("CRTC set but no FB\n");
> return -EINVAL;
> } else if (WARN_ON(state->fb && !state->crtc)) {
> - DRM_DEBUG_ATOMIC("FB set but no CRTC\n");
> + DRM_DEBUG_KMS("FB set but no CRTC\n");
> return -EINVAL;
> }
>
> @@ -815,15 +815,15 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
>
> /* Check whether this plane is usable on this CRTC */
> if (!(plane->possible_crtcs & drm_crtc_mask(state->crtc))) {
> - DRM_DEBUG_ATOMIC("Invalid crtc for plane\n");
> + DRM_DEBUG_KMS("Invalid crtc for plane\n");
> return -EINVAL;
> }
>
> /* Check whether this plane supports the fb pixel format. */
> ret = drm_plane_check_pixel_format(plane, state->fb->pixel_format);
> if (ret) {
> - DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
> - drm_get_format_name(state->fb->pixel_format));
> + DRM_DEBUG_KMS("Invalid pixel format %s\n",
> + drm_get_format_name(state->fb->pixel_format));
> return ret;
> }
>
> @@ -832,9 +832,9 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> state->crtc_x > INT_MAX - (int32_t) state->crtc_w ||
> state->crtc_h > INT_MAX ||
> state->crtc_y > INT_MAX - (int32_t) state->crtc_h) {
> - DRM_DEBUG_ATOMIC("Invalid CRTC coordinates %ux%u+%d+%d\n",
> - state->crtc_w, state->crtc_h,
> - state->crtc_x, state->crtc_y);
> + DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
> + state->crtc_w, state->crtc_h,
> + state->crtc_x, state->crtc_y);
> return -ERANGE;
> }
>
> @@ -846,18 +846,18 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> state->src_x > fb_width - state->src_w ||
> state->src_h > fb_height ||
> state->src_y > fb_height - state->src_h) {
> - DRM_DEBUG_ATOMIC("Invalid source coordinates "
> - "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
> - state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
> - state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
> - state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
> - state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10);
> + DRM_DEBUG_KMS("Invalid source coordinates "
> + "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
> + state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
> + state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
> + state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
> + state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10);
> return -ENOSPC;
> }
>
> if (plane_switching_crtc(state->state, plane, state)) {
> - DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
> - plane->base.id, plane->name);
> + DRM_DEBUG_KMS("[PLANE:%d:%s] switching CRTC directly\n",
> + plane->base.id, plane->name);
> return -EINVAL;
> }
>
> @@ -1330,8 +1330,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:%s] atomic core check failed\n",
> - plane->base.id, plane->name);
> + DRM_DEBUG_KMS("[PLANE:%d:%s] atomic core check failed\n",
> + plane->base.id, plane->name);
> return ret;
> }
> }
> @@ -1339,8 +1339,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:%s] atomic core check failed\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] atomic core check failed\n",
> + crtc->base.id, crtc->name);
> return ret;
> }
> }
> @@ -1351,8 +1351,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:%s] requires full modeset\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[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 43a0b3dfa846..18ebb1a08d6c 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -115,9 +115,9 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
>
> if (new_encoder) {
> if (encoder_mask & (1 << drm_encoder_index(new_encoder))) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
> - new_encoder->base.id, new_encoder->name,
> - connector->base.id, connector->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
> + new_encoder->base.id, new_encoder->name,
> + connector->base.id, connector->name);
>
> return -EINVAL;
> }
> @@ -151,11 +151,11 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
> continue;
>
> if (!disable_conflicting_encoders) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
> - encoder->base.id, encoder->name,
> - connector->state->crtc->base.id,
> - connector->state->crtc->name,
> - connector->base.id, connector->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
> + encoder->base.id, encoder->name,
> + connector->state->crtc->base.id,
> + connector->state->crtc->name,
> + connector->base.id, connector->name);
> return -EINVAL;
> }
>
> @@ -302,17 +302,17 @@ update_connector_routing(struct drm_atomic_state *state,
> new_encoder = funcs->best_encoder(connector);
>
> if (!new_encoder) {
> - DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> - connector->base.id,
> - connector->name);
> + DRM_DEBUG_KMS("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> + connector->base.id,
> + connector->name);
> return -EINVAL;
> }
>
> if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d]\n",
> - new_encoder->base.id,
> - new_encoder->name,
> - connector_state->crtc->base.id);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] incompatible with [CRTC:%d]\n",
> + new_encoder->base.id,
> + new_encoder->name,
> + connector_state->crtc->base.id);
> return -EINVAL;
> }
>
> @@ -388,7 +388,7 @@ mode_fixup(struct drm_atomic_state *state)
> ret = drm_bridge_mode_fixup(encoder->bridge, &crtc_state->mode,
> &crtc_state->adjusted_mode);
> if (!ret) {
> - DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
> + DRM_DEBUG_KMS("Bridge fixup failed\n");
> return -EINVAL;
> }
>
> @@ -396,16 +396,16 @@ mode_fixup(struct drm_atomic_state *state)
> ret = funcs->atomic_check(encoder, crtc_state,
> conn_state);
> if (ret) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n",
> - encoder->base.id, encoder->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] check failed\n",
> + encoder->base.id, encoder->name);
> return ret;
> }
> } else if (funcs && funcs->mode_fixup) {
> ret = funcs->mode_fixup(encoder, &crtc_state->mode,
> &crtc_state->adjusted_mode);
> if (!ret) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n",
> - encoder->base.id, encoder->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] fixup failed\n",
> + encoder->base.id, encoder->name);
> return -EINVAL;
> }
> }
> @@ -425,8 +425,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:%s] fixup failed\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] fixup failed\n",
> + crtc->base.id, crtc->name);
> return -EINVAL;
> }
> }
> @@ -549,8 +549,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> return ret;
>
> if (crtc_state->enable != has_connectors) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] enabled/connectors mismatch\n",
> + crtc->base.id, crtc->name);
>
> return -EINVAL;
> }
> @@ -597,8 +597,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
>
> ret = funcs->atomic_check(plane, plane_state);
> if (ret) {
> - DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n",
> - plane->base.id, plane->name);
> + DRM_DEBUG_KMS("[PLANE:%d:%s] atomic driver check failed\n",
> + plane->base.id, plane->name);
> return ret;
> }
> }
> @@ -613,8 +613,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:%s] atomic driver check failed\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] atomic driver check failed\n",
> + crtc->base.id, crtc->name);
> return ret;
> }
> }
> @@ -2367,8 +2367,8 @@ retry:
> /* Make sure we don't accidentally do a full modeset. */
> state->allow_modeset = false;
> if (!crtc_state->active) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
> - crtc->base.id);
> + DRM_DEBUG_KMS("[CRTC:%d] disabled, rejecting legacy flip\n",
> + crtc->base.id);
> ret = -EINVAL;
> goto fail;
> }
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths
2016-05-13 15:13 [PATCH] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths ville.syrjala
2016-05-16 7:47 ` Jani Nikula
@ 2016-05-16 12:21 ` Emil Velikov
2017-11-15 18:38 ` [PATCH v2] " Ville Syrjala
2 siblings, 0 replies; 6+ messages in thread
From: Emil Velikov @ 2016-05-16 12:21 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: ML dri-devel
On 13 May 2016 at 16:13, <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> DRM_DEBUG_ATOMIC generates a lot of noise that no one normally cares
> about. However error paths everyone cares about, so hiding thea error
> debugs under DRM_DEBUG_ATOMIC is a bad idea. Let's use DRM_DEBUG_KMS
> for those instead.
>
A nice alternative/parallel solution would be is to have tag/prefix
for the different types of messages. It will make things hell lot
easier to manage ;-)
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths
2016-05-13 15:13 [PATCH] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths ville.syrjala
2016-05-16 7:47 ` Jani Nikula
2016-05-16 12:21 ` Emil Velikov
@ 2017-11-15 18:38 ` Ville Syrjala
2017-11-24 16:13 ` Ville Syrjälä
2 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjala @ 2017-11-15 18:38 UTC (permalink / raw)
To: dri-devel; +Cc: Jani Nikula
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
DRM_DEBUG_ATOMIC generates a lot of noise that no one normally cares
about. However error paths everyone cares about, so hiding thea error
debugs under DRM_DEBUG_ATOMIC is a bad idea. Let's use DRM_DEBUG_KMS
for those instead.
v2: Rebase and handle a few new cases
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com> #v1
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 64 ++++++++++++++++-----------------
drivers/gpu/drm/drm_atomic_helper.c | 70 ++++++++++++++++++-------------------
2 files changed, 66 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 37445d50816a..594bdd5c33cb 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -572,8 +572,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
*/
if (state->active && !state->enable) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active without enabled\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] active without enabled\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -582,15 +582,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:%s] enabled without mode blob\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[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:%s] disabled with mode blob\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] disabled with mode blob\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -605,8 +605,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
* pipe.
*/
if (state->event && !state->active && !crtc->state->active) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requesting event but off\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] requesting event but off\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -861,10 +861,10 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
/* either *both* CRTC and FB must be set, or neither */
if (WARN_ON(state->crtc && !state->fb)) {
- DRM_DEBUG_ATOMIC("CRTC set but no FB\n");
+ DRM_DEBUG_KMS("CRTC set but no FB\n");
return -EINVAL;
} else if (WARN_ON(state->fb && !state->crtc)) {
- DRM_DEBUG_ATOMIC("FB set but no CRTC\n");
+ DRM_DEBUG_KMS("FB set but no CRTC\n");
return -EINVAL;
}
@@ -874,7 +874,7 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
/* Check whether this plane is usable on this CRTC */
if (!(plane->possible_crtcs & drm_crtc_mask(state->crtc))) {
- DRM_DEBUG_ATOMIC("Invalid crtc for plane\n");
+ DRM_DEBUG_KMS("Invalid crtc for plane\n");
return -EINVAL;
}
@@ -882,9 +882,9 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
ret = drm_plane_check_pixel_format(plane, state->fb->format->format);
if (ret) {
struct drm_format_name_buf format_name;
- DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
- drm_get_format_name(state->fb->format->format,
- &format_name));
+ DRM_DEBUG_KMS("Invalid pixel format %s\n",
+ drm_get_format_name(state->fb->format->format,
+ &format_name));
return ret;
}
@@ -893,9 +893,9 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
state->crtc_x > INT_MAX - (int32_t) state->crtc_w ||
state->crtc_h > INT_MAX ||
state->crtc_y > INT_MAX - (int32_t) state->crtc_h) {
- DRM_DEBUG_ATOMIC("Invalid CRTC coordinates %ux%u+%d+%d\n",
- state->crtc_w, state->crtc_h,
- state->crtc_x, state->crtc_y);
+ DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
+ state->crtc_w, state->crtc_h,
+ state->crtc_x, state->crtc_y);
return -ERANGE;
}
@@ -907,19 +907,19 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
state->src_x > fb_width - state->src_w ||
state->src_h > fb_height ||
state->src_y > fb_height - state->src_h) {
- DRM_DEBUG_ATOMIC("Invalid source coordinates "
- "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
- state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
- state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
- state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
- state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10,
- state->fb->width, state->fb->height);
+ DRM_DEBUG_KMS("Invalid source coordinates "
+ "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
+ state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
+ state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
+ state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
+ state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10,
+ state->fb->width, state->fb->height);
return -ENOSPC;
}
if (plane_switching_crtc(state->state, plane, state)) {
- DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
- plane->base.id, plane->name);
+ DRM_DEBUG_KMS("[PLANE:%d:%s] switching CRTC directly\n",
+ plane->base.id, plane->name);
return -EINVAL;
}
@@ -1596,8 +1596,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
for_each_new_plane_in_state(state, plane, plane_state, i) {
ret = drm_atomic_plane_check(plane, plane_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check failed\n",
- plane->base.id, plane->name);
+ DRM_DEBUG_KMS("[PLANE:%d:%s] atomic core check failed\n",
+ plane->base.id, plane->name);
return ret;
}
}
@@ -1605,8 +1605,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
ret = drm_atomic_crtc_check(crtc, crtc_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check failed\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] atomic core check failed\n",
+ crtc->base.id, crtc->name);
return ret;
}
}
@@ -1620,8 +1620,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
if (!state->allow_modeset) {
for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
if (drm_atomic_crtc_needs_modeset(crtc_state)) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full modeset\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[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 ced1ac8e68a0..8625b181f6b6 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -121,9 +121,9 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
if (new_encoder) {
if (encoder_mask & (1 << drm_encoder_index(new_encoder))) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
- new_encoder->base.id, new_encoder->name,
- connector->base.id, connector->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
+ new_encoder->base.id, new_encoder->name,
+ connector->base.id, connector->name);
return -EINVAL;
}
@@ -158,11 +158,11 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
continue;
if (!disable_conflicting_encoders) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
- encoder->base.id, encoder->name,
- connector->state->crtc->base.id,
- connector->state->crtc->name,
- connector->base.id, connector->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
+ encoder->base.id, encoder->name,
+ connector->state->crtc->base.id,
+ connector->state->crtc->name,
+ connector->base.id, connector->name);
ret = -EINVAL;
goto out;
}
@@ -317,18 +317,16 @@ update_connector_routing(struct drm_atomic_state *state,
new_encoder = drm_atomic_helper_best_encoder(connector);
if (!new_encoder) {
- DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
- connector->base.id,
- connector->name);
+ DRM_DEBUG_KMS("No suitable encoder found for [CONNECTOR:%d:%s]\n",
+ connector->base.id, connector->name);
return -EINVAL;
}
if (!drm_encoder_crtc_ok(new_encoder, new_connector_state->crtc)) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
- new_encoder->base.id,
- new_encoder->name,
- new_connector_state->crtc->base.id,
- new_connector_state->crtc->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
+ new_encoder->base.id, new_encoder->name,
+ new_connector_state->crtc->base.id,
+ new_connector_state->crtc->name);
return -EINVAL;
}
@@ -404,7 +402,7 @@ mode_fixup(struct drm_atomic_state *state)
ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state->mode,
&new_crtc_state->adjusted_mode);
if (!ret) {
- DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
+ DRM_DEBUG_KMS("Bridge fixup failed\n");
return -EINVAL;
}
@@ -412,16 +410,16 @@ mode_fixup(struct drm_atomic_state *state)
ret = funcs->atomic_check(encoder, new_crtc_state,
new_conn_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n",
- encoder->base.id, encoder->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] check failed\n",
+ encoder->base.id, encoder->name);
return ret;
}
} else if (funcs && funcs->mode_fixup) {
ret = funcs->mode_fixup(encoder, &new_crtc_state->mode,
&new_crtc_state->adjusted_mode);
if (!ret) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n",
- encoder->base.id, encoder->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] fixup failed\n",
+ encoder->base.id, encoder->name);
return -EINVAL;
}
}
@@ -444,8 +442,8 @@ mode_fixup(struct drm_atomic_state *state)
ret = funcs->mode_fixup(crtc, &new_crtc_state->mode,
&new_crtc_state->adjusted_mode);
if (!ret) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] fixup failed\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
}
@@ -462,21 +460,21 @@ static enum drm_mode_status mode_valid_path(struct drm_connector *connector,
ret = drm_encoder_mode_valid(encoder, mode);
if (ret != MODE_OK) {
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] mode_valid() failed\n",
- encoder->base.id, encoder->name);
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] mode_valid() failed\n",
+ encoder->base.id, encoder->name);
return ret;
}
ret = drm_bridge_mode_valid(encoder->bridge, mode);
if (ret != MODE_OK) {
- DRM_DEBUG_ATOMIC("[BRIDGE] mode_valid() failed\n");
+ DRM_DEBUG_KMS("[BRIDGE] mode_valid() failed\n");
return ret;
}
ret = drm_crtc_mode_valid(crtc, mode);
if (ret != MODE_OK) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode_valid() failed\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] mode_valid() failed\n",
+ crtc->base.id, crtc->name);
return ret;
}
@@ -605,8 +603,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
}
if (new_crtc_state->enable != has_connectors) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] enabled/connectors mismatch\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -735,8 +733,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
ret = funcs->atomic_check(plane, new_plane_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n",
- plane->base.id, plane->name);
+ DRM_DEBUG_KMS("[PLANE:%d:%s] atomic driver check failed\n",
+ plane->base.id, plane->name);
return ret;
}
}
@@ -751,8 +749,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
ret = funcs->atomic_check(crtc, new_crtc_state);
if (ret) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] atomic driver check failed\n",
+ crtc->base.id, crtc->name);
return ret;
}
}
@@ -3098,8 +3096,8 @@ static int page_flip_common(struct drm_atomic_state *state,
/* Make sure we don't accidentally do a full modeset. */
state->allow_modeset = false;
if (!crtc_state->active) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] disabled, rejecting legacy flip\n",
- crtc->base.id, crtc->name);
+ DRM_DEBUG_KMS("[CRTC:%d:%s] disabled, rejecting legacy flip\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
--
2.13.6
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths
2017-11-15 18:38 ` [PATCH v2] " Ville Syrjala
@ 2017-11-24 16:13 ` Ville Syrjälä
2017-11-27 10:14 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2017-11-24 16:13 UTC (permalink / raw)
To: dri-devel; +Cc: Jani Nikula
On Wed, Nov 15, 2017 at 08:38:41PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> DRM_DEBUG_ATOMIC generates a lot of noise that no one normally cares
> about. However error paths everyone cares about, so hiding thea error
> debugs under DRM_DEBUG_ATOMIC is a bad idea. Let's use DRM_DEBUG_KMS
> for those instead.
Actually, one idea that came to my mind right now is that maybe we want
to keep using _ATOMIC with TEST_ONLY, and only use _KMS w/o TEST_ONLY?
>
> v2: Rebase and handle a few new cases
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com> #v1
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 64 ++++++++++++++++-----------------
> drivers/gpu/drm/drm_atomic_helper.c | 70 ++++++++++++++++++-------------------
> 2 files changed, 66 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 37445d50816a..594bdd5c33cb 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -572,8 +572,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
> */
>
> if (state->active && !state->enable) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active without enabled\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] active without enabled\n",
> + crtc->base.id, crtc->name);
> return -EINVAL;
> }
>
> @@ -582,15 +582,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:%s] enabled without mode blob\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[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:%s] disabled with mode blob\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] disabled with mode blob\n",
> + crtc->base.id, crtc->name);
> return -EINVAL;
> }
>
> @@ -605,8 +605,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
> * pipe.
> */
> if (state->event && !state->active && !crtc->state->active) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requesting event but off\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] requesting event but off\n",
> + crtc->base.id, crtc->name);
> return -EINVAL;
> }
>
> @@ -861,10 +861,10 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
>
> /* either *both* CRTC and FB must be set, or neither */
> if (WARN_ON(state->crtc && !state->fb)) {
> - DRM_DEBUG_ATOMIC("CRTC set but no FB\n");
> + DRM_DEBUG_KMS("CRTC set but no FB\n");
> return -EINVAL;
> } else if (WARN_ON(state->fb && !state->crtc)) {
> - DRM_DEBUG_ATOMIC("FB set but no CRTC\n");
> + DRM_DEBUG_KMS("FB set but no CRTC\n");
> return -EINVAL;
> }
>
> @@ -874,7 +874,7 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
>
> /* Check whether this plane is usable on this CRTC */
> if (!(plane->possible_crtcs & drm_crtc_mask(state->crtc))) {
> - DRM_DEBUG_ATOMIC("Invalid crtc for plane\n");
> + DRM_DEBUG_KMS("Invalid crtc for plane\n");
> return -EINVAL;
> }
>
> @@ -882,9 +882,9 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> ret = drm_plane_check_pixel_format(plane, state->fb->format->format);
> if (ret) {
> struct drm_format_name_buf format_name;
> - DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
> - drm_get_format_name(state->fb->format->format,
> - &format_name));
> + DRM_DEBUG_KMS("Invalid pixel format %s\n",
> + drm_get_format_name(state->fb->format->format,
> + &format_name));
> return ret;
> }
>
> @@ -893,9 +893,9 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> state->crtc_x > INT_MAX - (int32_t) state->crtc_w ||
> state->crtc_h > INT_MAX ||
> state->crtc_y > INT_MAX - (int32_t) state->crtc_h) {
> - DRM_DEBUG_ATOMIC("Invalid CRTC coordinates %ux%u+%d+%d\n",
> - state->crtc_w, state->crtc_h,
> - state->crtc_x, state->crtc_y);
> + DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
> + state->crtc_w, state->crtc_h,
> + state->crtc_x, state->crtc_y);
> return -ERANGE;
> }
>
> @@ -907,19 +907,19 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> state->src_x > fb_width - state->src_w ||
> state->src_h > fb_height ||
> state->src_y > fb_height - state->src_h) {
> - DRM_DEBUG_ATOMIC("Invalid source coordinates "
> - "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
> - state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
> - state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
> - state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
> - state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10,
> - state->fb->width, state->fb->height);
> + DRM_DEBUG_KMS("Invalid source coordinates "
> + "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
> + state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
> + state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
> + state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
> + state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10,
> + state->fb->width, state->fb->height);
> return -ENOSPC;
> }
>
> if (plane_switching_crtc(state->state, plane, state)) {
> - DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
> - plane->base.id, plane->name);
> + DRM_DEBUG_KMS("[PLANE:%d:%s] switching CRTC directly\n",
> + plane->base.id, plane->name);
> return -EINVAL;
> }
>
> @@ -1596,8 +1596,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
> for_each_new_plane_in_state(state, plane, plane_state, i) {
> ret = drm_atomic_plane_check(plane, plane_state);
> if (ret) {
> - DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check failed\n",
> - plane->base.id, plane->name);
> + DRM_DEBUG_KMS("[PLANE:%d:%s] atomic core check failed\n",
> + plane->base.id, plane->name);
> return ret;
> }
> }
> @@ -1605,8 +1605,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
> for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> ret = drm_atomic_crtc_check(crtc, crtc_state);
> if (ret) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check failed\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] atomic core check failed\n",
> + crtc->base.id, crtc->name);
> return ret;
> }
> }
> @@ -1620,8 +1620,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
> if (!state->allow_modeset) {
> for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full modeset\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[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 ced1ac8e68a0..8625b181f6b6 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -121,9 +121,9 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
>
> if (new_encoder) {
> if (encoder_mask & (1 << drm_encoder_index(new_encoder))) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
> - new_encoder->base.id, new_encoder->name,
> - connector->base.id, connector->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
> + new_encoder->base.id, new_encoder->name,
> + connector->base.id, connector->name);
>
> return -EINVAL;
> }
> @@ -158,11 +158,11 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
> continue;
>
> if (!disable_conflicting_encoders) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
> - encoder->base.id, encoder->name,
> - connector->state->crtc->base.id,
> - connector->state->crtc->name,
> - connector->base.id, connector->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
> + encoder->base.id, encoder->name,
> + connector->state->crtc->base.id,
> + connector->state->crtc->name,
> + connector->base.id, connector->name);
> ret = -EINVAL;
> goto out;
> }
> @@ -317,18 +317,16 @@ update_connector_routing(struct drm_atomic_state *state,
> new_encoder = drm_atomic_helper_best_encoder(connector);
>
> if (!new_encoder) {
> - DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> - connector->base.id,
> - connector->name);
> + DRM_DEBUG_KMS("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> + connector->base.id, connector->name);
> return -EINVAL;
> }
>
> if (!drm_encoder_crtc_ok(new_encoder, new_connector_state->crtc)) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
> - new_encoder->base.id,
> - new_encoder->name,
> - new_connector_state->crtc->base.id,
> - new_connector_state->crtc->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
> + new_encoder->base.id, new_encoder->name,
> + new_connector_state->crtc->base.id,
> + new_connector_state->crtc->name);
> return -EINVAL;
> }
>
> @@ -404,7 +402,7 @@ mode_fixup(struct drm_atomic_state *state)
> ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state->mode,
> &new_crtc_state->adjusted_mode);
> if (!ret) {
> - DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
> + DRM_DEBUG_KMS("Bridge fixup failed\n");
> return -EINVAL;
> }
>
> @@ -412,16 +410,16 @@ mode_fixup(struct drm_atomic_state *state)
> ret = funcs->atomic_check(encoder, new_crtc_state,
> new_conn_state);
> if (ret) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n",
> - encoder->base.id, encoder->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] check failed\n",
> + encoder->base.id, encoder->name);
> return ret;
> }
> } else if (funcs && funcs->mode_fixup) {
> ret = funcs->mode_fixup(encoder, &new_crtc_state->mode,
> &new_crtc_state->adjusted_mode);
> if (!ret) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n",
> - encoder->base.id, encoder->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] fixup failed\n",
> + encoder->base.id, encoder->name);
> return -EINVAL;
> }
> }
> @@ -444,8 +442,8 @@ mode_fixup(struct drm_atomic_state *state)
> ret = funcs->mode_fixup(crtc, &new_crtc_state->mode,
> &new_crtc_state->adjusted_mode);
> if (!ret) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] fixup failed\n",
> + crtc->base.id, crtc->name);
> return -EINVAL;
> }
> }
> @@ -462,21 +460,21 @@ static enum drm_mode_status mode_valid_path(struct drm_connector *connector,
>
> ret = drm_encoder_mode_valid(encoder, mode);
> if (ret != MODE_OK) {
> - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] mode_valid() failed\n",
> - encoder->base.id, encoder->name);
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] mode_valid() failed\n",
> + encoder->base.id, encoder->name);
> return ret;
> }
>
> ret = drm_bridge_mode_valid(encoder->bridge, mode);
> if (ret != MODE_OK) {
> - DRM_DEBUG_ATOMIC("[BRIDGE] mode_valid() failed\n");
> + DRM_DEBUG_KMS("[BRIDGE] mode_valid() failed\n");
> return ret;
> }
>
> ret = drm_crtc_mode_valid(crtc, mode);
> if (ret != MODE_OK) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode_valid() failed\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] mode_valid() failed\n",
> + crtc->base.id, crtc->name);
> return ret;
> }
>
> @@ -605,8 +603,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> }
>
> if (new_crtc_state->enable != has_connectors) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] enabled/connectors mismatch\n",
> + crtc->base.id, crtc->name);
>
> return -EINVAL;
> }
> @@ -735,8 +733,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
>
> ret = funcs->atomic_check(plane, new_plane_state);
> if (ret) {
> - DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n",
> - plane->base.id, plane->name);
> + DRM_DEBUG_KMS("[PLANE:%d:%s] atomic driver check failed\n",
> + plane->base.id, plane->name);
> return ret;
> }
> }
> @@ -751,8 +749,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
>
> ret = funcs->atomic_check(crtc, new_crtc_state);
> if (ret) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] atomic driver check failed\n",
> + crtc->base.id, crtc->name);
> return ret;
> }
> }
> @@ -3098,8 +3096,8 @@ static int page_flip_common(struct drm_atomic_state *state,
> /* Make sure we don't accidentally do a full modeset. */
> state->allow_modeset = false;
> if (!crtc_state->active) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] disabled, rejecting legacy flip\n",
> - crtc->base.id, crtc->name);
> + DRM_DEBUG_KMS("[CRTC:%d:%s] disabled, rejecting legacy flip\n",
> + crtc->base.id, crtc->name);
> return -EINVAL;
> }
>
> --
> 2.13.6
--
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] 6+ messages in thread
* Re: [PATCH v2] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths
2017-11-24 16:13 ` Ville Syrjälä
@ 2017-11-27 10:14 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2017-11-27 10:14 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Jani Nikula, dri-devel
On Fri, Nov 24, 2017 at 06:13:56PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 15, 2017 at 08:38:41PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > DRM_DEBUG_ATOMIC generates a lot of noise that no one normally cares
> > about. However error paths everyone cares about, so hiding thea error
> > debugs under DRM_DEBUG_ATOMIC is a bad idea. Let's use DRM_DEBUG_KMS
> > for those instead.
>
> Actually, one idea that came to my mind right now is that maybe we want
> to keep using _ATOMIC with TEST_ONLY, and only use _KMS w/o TEST_ONLY?
Yeah a DRM_DEBUG_ATOMIC_CHECK which does this sounds like a very good
idea. I even thought of encapsulating the return -EINVAL, but that's maybe
a bit too nasty control flow :-) But maybe something like
#define atomic_check_failure_return(reason, ..)
wouldn't be too eye-gauging ...
-Daniel
>
> >
> > v2: Rebase and handle a few new cases
> >
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com> #v1
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 64 ++++++++++++++++-----------------
> > drivers/gpu/drm/drm_atomic_helper.c | 70 ++++++++++++++++++-------------------
> > 2 files changed, 66 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 37445d50816a..594bdd5c33cb 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -572,8 +572,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
> > */
> >
> > if (state->active && !state->enable) {
> > - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active without enabled\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[CRTC:%d:%s] active without enabled\n",
> > + crtc->base.id, crtc->name);
> > return -EINVAL;
> > }
> >
> > @@ -582,15 +582,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:%s] enabled without mode blob\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[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:%s] disabled with mode blob\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[CRTC:%d:%s] disabled with mode blob\n",
> > + crtc->base.id, crtc->name);
> > return -EINVAL;
> > }
> >
> > @@ -605,8 +605,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
> > * pipe.
> > */
> > if (state->event && !state->active && !crtc->state->active) {
> > - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requesting event but off\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[CRTC:%d:%s] requesting event but off\n",
> > + crtc->base.id, crtc->name);
> > return -EINVAL;
> > }
> >
> > @@ -861,10 +861,10 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> >
> > /* either *both* CRTC and FB must be set, or neither */
> > if (WARN_ON(state->crtc && !state->fb)) {
> > - DRM_DEBUG_ATOMIC("CRTC set but no FB\n");
> > + DRM_DEBUG_KMS("CRTC set but no FB\n");
> > return -EINVAL;
> > } else if (WARN_ON(state->fb && !state->crtc)) {
> > - DRM_DEBUG_ATOMIC("FB set but no CRTC\n");
> > + DRM_DEBUG_KMS("FB set but no CRTC\n");
> > return -EINVAL;
> > }
> >
> > @@ -874,7 +874,7 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> >
> > /* Check whether this plane is usable on this CRTC */
> > if (!(plane->possible_crtcs & drm_crtc_mask(state->crtc))) {
> > - DRM_DEBUG_ATOMIC("Invalid crtc for plane\n");
> > + DRM_DEBUG_KMS("Invalid crtc for plane\n");
> > return -EINVAL;
> > }
> >
> > @@ -882,9 +882,9 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> > ret = drm_plane_check_pixel_format(plane, state->fb->format->format);
> > if (ret) {
> > struct drm_format_name_buf format_name;
> > - DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
> > - drm_get_format_name(state->fb->format->format,
> > - &format_name));
> > + DRM_DEBUG_KMS("Invalid pixel format %s\n",
> > + drm_get_format_name(state->fb->format->format,
> > + &format_name));
> > return ret;
> > }
> >
> > @@ -893,9 +893,9 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> > state->crtc_x > INT_MAX - (int32_t) state->crtc_w ||
> > state->crtc_h > INT_MAX ||
> > state->crtc_y > INT_MAX - (int32_t) state->crtc_h) {
> > - DRM_DEBUG_ATOMIC("Invalid CRTC coordinates %ux%u+%d+%d\n",
> > - state->crtc_w, state->crtc_h,
> > - state->crtc_x, state->crtc_y);
> > + DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
> > + state->crtc_w, state->crtc_h,
> > + state->crtc_x, state->crtc_y);
> > return -ERANGE;
> > }
> >
> > @@ -907,19 +907,19 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> > state->src_x > fb_width - state->src_w ||
> > state->src_h > fb_height ||
> > state->src_y > fb_height - state->src_h) {
> > - DRM_DEBUG_ATOMIC("Invalid source coordinates "
> > - "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
> > - state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
> > - state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
> > - state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
> > - state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10,
> > - state->fb->width, state->fb->height);
> > + DRM_DEBUG_KMS("Invalid source coordinates "
> > + "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
> > + state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,
> > + state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,
> > + state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,
> > + state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10,
> > + state->fb->width, state->fb->height);
> > return -ENOSPC;
> > }
> >
> > if (plane_switching_crtc(state->state, plane, state)) {
> > - DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
> > - plane->base.id, plane->name);
> > + DRM_DEBUG_KMS("[PLANE:%d:%s] switching CRTC directly\n",
> > + plane->base.id, plane->name);
> > return -EINVAL;
> > }
> >
> > @@ -1596,8 +1596,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
> > for_each_new_plane_in_state(state, plane, plane_state, i) {
> > ret = drm_atomic_plane_check(plane, plane_state);
> > if (ret) {
> > - DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check failed\n",
> > - plane->base.id, plane->name);
> > + DRM_DEBUG_KMS("[PLANE:%d:%s] atomic core check failed\n",
> > + plane->base.id, plane->name);
> > return ret;
> > }
> > }
> > @@ -1605,8 +1605,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
> > for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > ret = drm_atomic_crtc_check(crtc, crtc_state);
> > if (ret) {
> > - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check failed\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[CRTC:%d:%s] atomic core check failed\n",
> > + crtc->base.id, crtc->name);
> > return ret;
> > }
> > }
> > @@ -1620,8 +1620,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
> > if (!state->allow_modeset) {
> > for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> > - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full modeset\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[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 ced1ac8e68a0..8625b181f6b6 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -121,9 +121,9 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
> >
> > if (new_encoder) {
> > if (encoder_mask & (1 << drm_encoder_index(new_encoder))) {
> > - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
> > - new_encoder->base.id, new_encoder->name,
> > - connector->base.id, connector->name);
> > + DRM_DEBUG_KMS("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
> > + new_encoder->base.id, new_encoder->name,
> > + connector->base.id, connector->name);
> >
> > return -EINVAL;
> > }
> > @@ -158,11 +158,11 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
> > continue;
> >
> > if (!disable_conflicting_encoders) {
> > - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
> > - encoder->base.id, encoder->name,
> > - connector->state->crtc->base.id,
> > - connector->state->crtc->name,
> > - connector->base.id, connector->name);
> > + DRM_DEBUG_KMS("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
> > + encoder->base.id, encoder->name,
> > + connector->state->crtc->base.id,
> > + connector->state->crtc->name,
> > + connector->base.id, connector->name);
> > ret = -EINVAL;
> > goto out;
> > }
> > @@ -317,18 +317,16 @@ update_connector_routing(struct drm_atomic_state *state,
> > new_encoder = drm_atomic_helper_best_encoder(connector);
> >
> > if (!new_encoder) {
> > - DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> > - connector->base.id,
> > - connector->name);
> > + DRM_DEBUG_KMS("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> > + connector->base.id, connector->name);
> > return -EINVAL;
> > }
> >
> > if (!drm_encoder_crtc_ok(new_encoder, new_connector_state->crtc)) {
> > - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
> > - new_encoder->base.id,
> > - new_encoder->name,
> > - new_connector_state->crtc->base.id,
> > - new_connector_state->crtc->name);
> > + DRM_DEBUG_KMS("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
> > + new_encoder->base.id, new_encoder->name,
> > + new_connector_state->crtc->base.id,
> > + new_connector_state->crtc->name);
> > return -EINVAL;
> > }
> >
> > @@ -404,7 +402,7 @@ mode_fixup(struct drm_atomic_state *state)
> > ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state->mode,
> > &new_crtc_state->adjusted_mode);
> > if (!ret) {
> > - DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
> > + DRM_DEBUG_KMS("Bridge fixup failed\n");
> > return -EINVAL;
> > }
> >
> > @@ -412,16 +410,16 @@ mode_fixup(struct drm_atomic_state *state)
> > ret = funcs->atomic_check(encoder, new_crtc_state,
> > new_conn_state);
> > if (ret) {
> > - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n",
> > - encoder->base.id, encoder->name);
> > + DRM_DEBUG_KMS("[ENCODER:%d:%s] check failed\n",
> > + encoder->base.id, encoder->name);
> > return ret;
> > }
> > } else if (funcs && funcs->mode_fixup) {
> > ret = funcs->mode_fixup(encoder, &new_crtc_state->mode,
> > &new_crtc_state->adjusted_mode);
> > if (!ret) {
> > - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n",
> > - encoder->base.id, encoder->name);
> > + DRM_DEBUG_KMS("[ENCODER:%d:%s] fixup failed\n",
> > + encoder->base.id, encoder->name);
> > return -EINVAL;
> > }
> > }
> > @@ -444,8 +442,8 @@ mode_fixup(struct drm_atomic_state *state)
> > ret = funcs->mode_fixup(crtc, &new_crtc_state->mode,
> > &new_crtc_state->adjusted_mode);
> > if (!ret) {
> > - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[CRTC:%d:%s] fixup failed\n",
> > + crtc->base.id, crtc->name);
> > return -EINVAL;
> > }
> > }
> > @@ -462,21 +460,21 @@ static enum drm_mode_status mode_valid_path(struct drm_connector *connector,
> >
> > ret = drm_encoder_mode_valid(encoder, mode);
> > if (ret != MODE_OK) {
> > - DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] mode_valid() failed\n",
> > - encoder->base.id, encoder->name);
> > + DRM_DEBUG_KMS("[ENCODER:%d:%s] mode_valid() failed\n",
> > + encoder->base.id, encoder->name);
> > return ret;
> > }
> >
> > ret = drm_bridge_mode_valid(encoder->bridge, mode);
> > if (ret != MODE_OK) {
> > - DRM_DEBUG_ATOMIC("[BRIDGE] mode_valid() failed\n");
> > + DRM_DEBUG_KMS("[BRIDGE] mode_valid() failed\n");
> > return ret;
> > }
> >
> > ret = drm_crtc_mode_valid(crtc, mode);
> > if (ret != MODE_OK) {
> > - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode_valid() failed\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[CRTC:%d:%s] mode_valid() failed\n",
> > + crtc->base.id, crtc->name);
> > return ret;
> > }
> >
> > @@ -605,8 +603,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > }
> >
> > if (new_crtc_state->enable != has_connectors) {
> > - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[CRTC:%d:%s] enabled/connectors mismatch\n",
> > + crtc->base.id, crtc->name);
> >
> > return -EINVAL;
> > }
> > @@ -735,8 +733,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> >
> > ret = funcs->atomic_check(plane, new_plane_state);
> > if (ret) {
> > - DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n",
> > - plane->base.id, plane->name);
> > + DRM_DEBUG_KMS("[PLANE:%d:%s] atomic driver check failed\n",
> > + plane->base.id, plane->name);
> > return ret;
> > }
> > }
> > @@ -751,8 +749,8 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> >
> > ret = funcs->atomic_check(crtc, new_crtc_state);
> > if (ret) {
> > - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[CRTC:%d:%s] atomic driver check failed\n",
> > + crtc->base.id, crtc->name);
> > return ret;
> > }
> > }
> > @@ -3098,8 +3096,8 @@ static int page_flip_common(struct drm_atomic_state *state,
> > /* Make sure we don't accidentally do a full modeset. */
> > state->allow_modeset = false;
> > if (!crtc_state->active) {
> > - DRM_DEBUG_ATOMIC("[CRTC:%d:%s] disabled, rejecting legacy flip\n",
> > - crtc->base.id, crtc->name);
> > + DRM_DEBUG_KMS("[CRTC:%d:%s] disabled, rejecting legacy flip\n",
> > + crtc->base.id, crtc->name);
> > return -EINVAL;
> > }
> >
> > --
> > 2.13.6
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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] 6+ messages in thread
end of thread, other threads:[~2017-11-27 10:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-13 15:13 [PATCH] drm/atomic: Use DRM_DEBUG_KMS instead of DRM_DEBUG_ATOMIC in error paths ville.syrjala
2016-05-16 7:47 ` Jani Nikula
2016-05-16 12:21 ` Emil Velikov
2017-11-15 18:38 ` [PATCH v2] " Ville Syrjala
2017-11-24 16:13 ` Ville Syrjälä
2017-11-27 10:14 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.