Re: [PATCH 1/3] drm: Add debug prints for the various object lookup errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 22, 2019 at 10:38:30AM +0100, Daniel Vetter wrote:
> On Mon, Jan 21, 2019 at 10:24:28PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > 
> > 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@xxxxxxxxxxxxxxx>
> 
> 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.

I suppose. Downside being that the debug then won't tell you who called
the _find(). Although with the atomic ioctl we really don't have much
information anyway since we didn't include the type of the object in the
ioctl struct. Perhaps that was a mistake.

> 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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux