[PATCH v2] drm/crtc: Remove duplicated ioctl code

[PATCH v2] drm/crtc: Remove duplicated ioctl code

[Chuck Ebbert]

Make drm_mode_add_fb() call drm_mode_add_fb2() after converting its
args to the new internal format, instead of duplicating code.

Also picks up a lot more error checking, which the legacy modes
should pass after being converted to the new format.

Signed-off-by: Chuck Ebbert <cebbert.lkml@gmail.com>
---
 drm_crtc.c |   31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

Against 3.17.0, compile tested only.
This time with the correct signed-off-by.

diff a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2925,11 +2925,9 @@ int drm_mode_addfb(struct drm_device *dev,
 {
 	struct drm_mode_fb_cmd *or = data;
 	struct drm_mode_fb_cmd2 r = {};
-	struct drm_mode_config *config = &dev->mode_config;
-	struct drm_framebuffer *fb;
-	int ret = 0;
+	int ret;
 
-	/* Use new struct with format internally */
+	/* convert to new format and call new ioctl */
 	r.fb_id = or->fb_id;
 	r.width = or->width;
 	r.height = or->height;
@@ -2937,27 +2934,12 @@ int drm_mode_addfb(struct drm_device *dev,
 	r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth);
 	r.handles[0] = or->handle;
 
-	if (!drm_core_check_feature(dev, DRIVER_MODESET))
-		return -EINVAL;
-
-	if ((config->min_width > r.width) || (r.width > config->max_width))
-		return -EINVAL;
-
-	if ((config->min_height > r.height) || (r.height > config->max_height))
-		return -EINVAL;
-
-	fb = dev->mode_config.funcs->fb_create(dev, file_priv, &r);
-	if (IS_ERR(fb)) {
-		DRM_DEBUG_KMS("could not create framebuffer\n");
-		return PTR_ERR(fb);
-	}
+	ret = drm_mode_addfb2(dev, &r, file_priv);
+	if (ret)
+		return ret;
+
+	or->fb_id = r.fb_id;
 
-	mutex_lock(&file_priv->fbs_lock);
-	or->fb_id = fb->base.id;
-	list_add(&fb->filp_head, &file_priv->fbs);
-	DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id);
-	mutex_unlock(&file_priv->fbs_lock);
-
 	return ret;
 }

Re: [PATCH v2] drm/crtc: Remove duplicated ioctl code

[Ville Syrjälä]

On Wed, Oct 08, 2014 at 11:40:34AM -0500, Chuck Ebbert wrote:
> Make drm_mode_add_fb() call drm_mode_add_fb2() after converting its
> args to the new internal format, instead of duplicating code.
> 
> Also picks up a lot more error checking, which the legacy modes
> should pass after being converted to the new format.
> 
> Signed-off-by: Chuck Ebbert <cebbert.lkml@gmail.com>

Looks good to me.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drm_crtc.c |   31 +++++++------------------------
>  1 file changed, 7 insertions(+), 24 deletions(-)
> 
> Against 3.17.0, compile tested only.
> This time with the correct signed-off-by.
> 
> diff a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2925,11 +2925,9 @@ int drm_mode_addfb(struct drm_device *dev,
>  {
>  	struct drm_mode_fb_cmd *or = data;
>  	struct drm_mode_fb_cmd2 r = {};
> -	struct drm_mode_config *config = &dev->mode_config;
> -	struct drm_framebuffer *fb;
> -	int ret = 0;
> +	int ret;
>  
> -	/* Use new struct with format internally */
> +	/* convert to new format and call new ioctl */
>  	r.fb_id = or->fb_id;
>  	r.width = or->width;
>  	r.height = or->height;
> @@ -2937,27 +2934,12 @@ int drm_mode_addfb(struct drm_device *dev,
>  	r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth);
>  	r.handles[0] = or->handle;
>  
> -	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> -		return -EINVAL;
> -
> -	if ((config->min_width > r.width) || (r.width > config->max_width))
> -		return -EINVAL;
> -
> -	if ((config->min_height > r.height) || (r.height > config->max_height))
> -		return -EINVAL;
> -
> -	fb = dev->mode_config.funcs->fb_create(dev, file_priv, &r);
> -	if (IS_ERR(fb)) {
> -		DRM_DEBUG_KMS("could not create framebuffer\n");
> -		return PTR_ERR(fb);
> -	}
> +	ret = drm_mode_addfb2(dev, &r, file_priv);
> +	if (ret)
> +		return ret;
> +
> +	or->fb_id = r.fb_id;
>  
> -	mutex_lock(&file_priv->fbs_lock);
> -	or->fb_id = fb->base.id;
> -	list_add(&fb->filp_head, &file_priv->fbs);
> -	DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id);
> -	mutex_unlock(&file_priv->fbs_lock);
> -
>  	return ret;
>  }
>  
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v2] drm/crtc: Remove duplicated ioctl code

[Daniel Vetter]

On Tue, Oct 21, 2014 at 05:08:10PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 08, 2014 at 11:40:34AM -0500, Chuck Ebbert wrote:
> > Make drm_mode_add_fb() call drm_mode_add_fb2() after converting its
> > args to the new internal format, instead of duplicating code.
> > 
> > Also picks up a lot more error checking, which the legacy modes
> > should pass after being converted to the new format.
> > 
> > Signed-off-by: Chuck Ebbert <cebbert.lkml@gmail.com>
> 
> Looks good to me.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Picked up into my drm core stuff queue.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch