From: Daniel Vetter <daniel@ffwll.ch>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/3] drm: Add debug prints for the various object lookup errors
Date: Tue, 22 Jan 2019 10:38:30 +0100 [thread overview]
Message-ID: <20190122093830.GW3271@phenom.ffwll.local> (raw)
In-Reply-To: <20190121202430.30789-1-ville.syrjala@linux.intel.com>
On Mon, Jan 21, 2019 at 10:24:28PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Only some of the drm mode object lookups have a corresponding debug
> print for the lookup failure. That makes logs a bit hard to parse
> when you can't see where the bad object ID is being used. Add a bunch
> more debug prints, and unify their appearance.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Instead of sprinkling these all over, what about the reverse route and
pushing this into drm_mode_object_find? We can dump id + object type, that
should be all we need really. If we go this way maybe add kerneldoc to the
various drm_*_find/lookup functions so this doesn't get copypasted again
...
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 5 +++++
> drivers/gpu/drm/drm_color_mgmt.c | 8 ++++++--
> drivers/gpu/drm/drm_connector.c | 5 ++++-
> drivers/gpu/drm/drm_crtc.c | 12 +++++++-----
> drivers/gpu/drm/drm_encoder.c | 4 +++-
> drivers/gpu/drm/drm_framebuffer.c | 4 +++-
> drivers/gpu/drm/drm_mode_object.c | 17 ++++++++++++++---
> drivers/gpu/drm/drm_plane.c | 13 +++++++++----
> drivers/gpu/drm/drm_property.c | 12 +++++++++---
> drivers/gpu/drm/drm_vblank.c | 8 ++++++--
> 10 files changed, 66 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 9a1f41adfc67..06390307e5a3 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1321,11 +1321,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>
> obj = drm_mode_object_find(dev, file_priv, obj_id, DRM_MODE_OBJECT_ANY);
> if (!obj) {
> + DRM_DEBUG_ATOMIC("Unknown object ID %d\n", obj_id);
> ret = -ENOENT;
> goto out;
> }
>
> if (!obj->properties) {
> + DRM_DEBUG_ATOMIC("Object ID %d has no properties\n",
> + obj_id);
> drm_mode_object_put(obj);
> ret = -ENOENT;
> goto out;
> @@ -1352,6 +1355,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>
> prop = drm_mode_obj_find_prop_id(obj, prop_id);
> if (!prop) {
> + DRM_DEBUG_ATOMIC("Unknown property ID %d\n",
> + prop_id);
> drm_mode_object_put(obj);
> ret = -ENOENT;
> goto out;
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 07dcf47daafe..a99ee15b8328 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -245,8 +245,10 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
> return -EOPNOTSUPP;
>
> crtc = drm_crtc_find(dev, file_priv, crtc_lut->crtc_id);
> - if (!crtc)
> + if (!crtc) {
> + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_lut->crtc_id);
> return -ENOENT;
> + }
>
> if (crtc->funcs->gamma_set == NULL)
> return -ENOSYS;
> @@ -313,8 +315,10 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev,
> return -EOPNOTSUPP;
>
> crtc = drm_crtc_find(dev, file_priv, crtc_lut->crtc_id);
> - if (!crtc)
> + if (!crtc) {
> + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_lut->crtc_id);
> return -ENOENT;
> + }
>
> /* memcpy into gamma store */
> if (crtc_lut->gamma_size != crtc->gamma_size)
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 847539645558..8745eb132fd4 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1952,8 +1952,11 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo));
>
> connector = drm_connector_lookup(dev, file_priv, out_resp->connector_id);
> - if (!connector)
> + if (!connector) {
> + DRM_DEBUG_KMS("Unknown connector ID %d\n",
> + out_resp->connector_id);
> return -ENOENT;
> + }
>
> drm_connector_for_each_possible_encoder(connector, encoder, i)
> encoders_count++;
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 7dabbaf033a1..e5f234ffcd23 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -369,8 +369,10 @@ int drm_mode_getcrtc(struct drm_device *dev,
> return -EOPNOTSUPP;
>
> crtc = drm_crtc_find(dev, file_priv, crtc_resp->crtc_id);
> - if (!crtc)
> + if (!crtc) {
> + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_resp->crtc_id);
> return -ENOENT;
> + }
>
> plane = crtc->primary;
>
> @@ -586,8 +588,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
> } else {
> fb = drm_framebuffer_lookup(dev, file_priv, crtc_req->fb_id);
> if (!fb) {
> - DRM_DEBUG_KMS("Unknown FB ID%d\n",
> - crtc_req->fb_id);
> + DRM_DEBUG_KMS("Unknown FB ID %d\n",
> + crtc_req->fb_id);
> ret = -ENOENT;
> goto out;
> }
> @@ -682,8 +684,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>
> connector = drm_connector_lookup(dev, file_priv, out_id);
> if (!connector) {
> - DRM_DEBUG_KMS("Connector id %d unknown\n",
> - out_id);
> + DRM_DEBUG_KMS("Unknown connector ID %d\n",
> + out_id);
> ret = -ENOENT;
> goto out;
> }
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index b694fb57eaa4..107f3fff3f3f 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -225,8 +225,10 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
> return -EOPNOTSUPP;
>
> encoder = drm_encoder_find(dev, file_priv, enc_resp->encoder_id);
> - if (!encoder)
> + if (!encoder) {
> + DRM_DEBUG_KMS("Unknown encoder ID %d\n", enc_resp->encoder_id);
> return -ENOENT;
> + }
>
> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> crtc = drm_encoder_get_crtc(encoder);
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 7abcb265a108..54f36bf7631e 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -431,8 +431,10 @@ int drm_mode_rmfb(struct drm_device *dev, u32 fb_id,
> return -EOPNOTSUPP;
>
> fb = drm_framebuffer_lookup(dev, file_priv, fb_id);
> - if (!fb)
> + if (!fb) {
> + DRM_DEBUG_KMS("Unknown FB ID %d\n", fb_id);
> return -ENOENT;
> + }
>
> mutex_lock(&file_priv->fbs_lock);
> list_for_each_entry(fbl, &file_priv->fbs, filp_head)
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index a9005c1c2384..e8dac94d576d 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -388,10 +388,13 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
>
> obj = drm_mode_object_find(dev, file_priv, arg->obj_id, arg->obj_type);
> if (!obj) {
> + DRM_DEBUG_KMS("Unknown object ID %d (type 0x%x)\n",
> + arg->obj_id, arg->obj_type);
> ret = -ENOENT;
> goto out;
> }
> if (!obj->properties) {
> + DRM_DEBUG_KMS("Object ID %d has no properties\n", arg->obj_id);
> ret = -EINVAL;
> goto out_unref;
> }
> @@ -509,15 +512,23 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
> return -EOPNOTSUPP;
>
> arg_obj = drm_mode_object_find(dev, file_priv, arg->obj_id, arg->obj_type);
> - if (!arg_obj)
> + if (!arg_obj) {
> + DRM_DEBUG_KMS("Unknown object ID %d (type 0x%x)\n",
> + arg->obj_id, arg->obj_type);
> return -ENOENT;
> + }
>
> - if (!arg_obj->properties)
> + if (!arg_obj->properties) {
> + DRM_DEBUG_KMS("Object ID %d has no properties\n",
> + arg->prop_id);
> goto out_unref;
> + }
>
> property = drm_mode_obj_find_prop_id(arg_obj, arg->prop_id);
> - if (!property)
> + if (!property) {
> + DRM_DEBUG_KMS("Unknown property ID %d\n", arg->prop_id);
> goto out_unref;
> + }
>
> if (drm_drv_uses_atomic_modeset(property->dev))
> ret = set_property_atomic(arg_obj, property, arg->value);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 4cfb56893b7f..830af4c60d55 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -520,8 +520,10 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
> return -EOPNOTSUPP;
>
> plane = drm_plane_find(dev, file_priv, plane_resp->plane_id);
> - if (!plane)
> + if (!plane) {
> + DRM_DEBUG_KMS("Unknown plane ID %d\n", plane_resp->plane_id);
> return -ENOENT;
> + }
>
> drm_modeset_lock(&plane->mutex, NULL);
> if (plane->state && plane->state->crtc && drm_lease_held(file_priv, plane->state->crtc->base.id))
> @@ -813,7 +815,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
> if (plane_req->fb_id) {
> fb = drm_framebuffer_lookup(dev, file_priv, plane_req->fb_id);
> if (!fb) {
> - DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
> + DRM_DEBUG_KMS("Unknown FB ID %d\n",
> plane_req->fb_id);
> return -ENOENT;
> }
> @@ -821,7 +823,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
> crtc = drm_crtc_find(dev, file_priv, plane_req->crtc_id);
> if (!crtc) {
> drm_framebuffer_put(fb);
> - DRM_DEBUG_KMS("Unknown crtc ID %d\n",
> + DRM_DEBUG_KMS("Unknown CRTC ID %d\n",
> plane_req->crtc_id);
> return -ENOENT;
> }
> @@ -1057,8 +1059,10 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> return -EINVAL;
>
> crtc = drm_crtc_find(dev, file_priv, page_flip->crtc_id);
> - if (!crtc)
> + if (!crtc) {
> + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", page_flip->crtc_id);
> return -ENOENT;
> + }
>
> plane = crtc->primary;
>
> @@ -1126,6 +1130,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>
> fb = drm_framebuffer_lookup(dev, file_priv, page_flip->fb_id);
> if (!fb) {
> + DRM_DEBUG_KMS("Unknown FB ID %d\n", page_flip->fb_id);
> ret = -ENOENT;
> goto out;
> }
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index 79c77c3cad86..254b71493221 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -467,8 +467,10 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
> return -EOPNOTSUPP;
>
> property = drm_property_find(dev, file_priv, out_resp->prop_id);
> - if (!property)
> + if (!property) {
> + DRM_DEBUG_KMS("Unknwon property ID %d\n", out_resp->prop_id);
> return -ENOENT;
> + }
>
> strncpy(out_resp->name, property->name, DRM_PROP_NAME_LEN);
> out_resp->name[DRM_PROP_NAME_LEN-1] = 0;
> @@ -760,8 +762,10 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
> return -EOPNOTSUPP;
>
> blob = drm_property_lookup_blob(dev, out_resp->blob_id);
> - if (!blob)
> + if (!blob) {
> + DRM_DEBUG_KMS("Unknwon blob ID %d\n", out_resp->blob_id);
> return -ENOENT;
> + }
>
> if (out_resp->length == blob->length) {
> if (copy_to_user(u64_to_user_ptr(out_resp->data),
> @@ -826,8 +830,10 @@ int drm_mode_destroyblob_ioctl(struct drm_device *dev,
> return -EOPNOTSUPP;
>
> blob = drm_property_lookup_blob(dev, out_resp->blob_id);
> - if (!blob)
> + if (!blob) {
> + DRM_DEBUG_KMS("Unknwon blob ID %d\n", out_resp->blob_id);
> return -ENOENT;
> + }
>
> mutex_lock(&dev->mode_config.blob_lock);
> /* Ensure the property was actually created by this user. */
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index cde71ee95a8f..793acfa5002e 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1816,8 +1816,10 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
> return -EINVAL;
>
> crtc = drm_crtc_find(dev, file_priv, get_seq->crtc_id);
> - if (!crtc)
> + if (!crtc) {
> + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", get_seq->crtc_id);
> return -ENOENT;
> + }
>
> pipe = drm_crtc_index(crtc);
>
> @@ -1874,8 +1876,10 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
> return -EINVAL;
>
> crtc = drm_crtc_find(dev, file_priv, queue_seq->crtc_id);
> - if (!crtc)
> + if (!crtc) {
> + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", queue_seq->crtc_id);
> return -ENOENT;
> + }
>
> flags = queue_seq->flags;
> /* Check valid flag bits */
> --
> 2.19.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-01-22 9:38 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-21 20:24 [PATCH 1/3] drm: Add debug prints for the various object lookup errors Ville Syrjala
2019-01-21 20:24 ` [PATCH 2/3] drm: Sync errno values for property " Ville Syrjala
2019-01-22 9:39 ` Daniel Vetter
2019-01-25 21:01 ` Ville Syrjälä
2019-01-29 8:49 ` Daniel Vetter
2019-02-15 8:21 ` [LKP] [drm] ef3de4356d: piglit.igt.kms_atomic.atomic_invalid_params.fail kernel test robot
2019-02-15 8:21 ` kernel test robot
2019-01-21 20:24 ` [PATCH 3/3] drm: Add a debug print for drm_modeset_backoff() Ville Syrjala
2019-01-22 9:41 ` Daniel Vetter
2019-01-21 21:07 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm: Add debug prints for the various object lookup errors Patchwork
2019-01-21 23:04 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-01-22 9:38 ` Daniel Vetter [this message]
2019-01-25 20:58 ` [Intel-gfx] [PATCH 1/3] " Ville Syrjälä
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190122093830.GW3271@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.