Re: [PATCH] drm/i915: Fix plane init failure paths

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Mar 21, 2016 at 02:43:22PM +0000, Matthew Auld wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Deal with errors from drm_universal_plane_init() in primary and cursor
> plane init paths (sprites were already covered). Also make the code
> neater by using goto for error handling.
> 
> v2: Rebased due to drm_universal_plane_init() 'name' parameter
> v3: Another rebase due to s/""/NULL/
> v4: Rebased on drm-nightly (Matthew Auld)
> v5: Fix email address (Matthew Auld)
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Pushed to dinq. Thanks for the review.

> ---
>  drivers/gpu/drm/i915/intel_display.c | 64 ++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_sprite.c  | 34 +++++++++++--------
>  2 files changed, 60 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 74b0165..f3f6fd1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13961,20 +13961,19 @@ const struct drm_plane_funcs intel_plane_funcs = {
>  static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  						    int pipe)
>  {
> -	struct intel_plane *primary;
> -	struct intel_plane_state *state;
> +	struct intel_plane *primary = NULL;
> +	struct intel_plane_state *state = NULL;
>  	const uint32_t *intel_primary_formats;
>  	unsigned int num_formats;
> +	int ret;
>  
>  	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
> -	if (primary == NULL)
> -		return NULL;
> +	if (!primary)
> +		goto fail;
>  
>  	state = intel_create_plane_state(&primary->base);
> -	if (!state) {
> -		kfree(primary);
> -		return NULL;
> -	}
> +	if (!state)
> +		goto fail;
>  	primary->base.state = &state->base;
>  
>  	primary->can_scale = false;
> @@ -14016,10 +14015,12 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  		primary->disable_plane = i9xx_disable_primary_plane;
>  	}
>  
> -	drm_universal_plane_init(dev, &primary->base, 0,
> -				 &intel_plane_funcs,
> -				 intel_primary_formats, num_formats,
> -				 DRM_PLANE_TYPE_PRIMARY, NULL);
> +	ret = drm_universal_plane_init(dev, &primary->base, 0,
> +				       &intel_plane_funcs,
> +				       intel_primary_formats, num_formats,
> +				       DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (ret)
> +		goto fail;
>  
>  	if (INTEL_INFO(dev)->gen >= 4)
>  		intel_create_rotation_property(dev, primary);
> @@ -14027,6 +14028,12 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
>  
>  	return &primary->base;
> +
> +fail:
> +	kfree(state);
> +	kfree(primary);
> +
> +	return NULL;
>  }
>  
>  void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane)
> @@ -14143,18 +14150,17 @@ intel_update_cursor_plane(struct drm_plane *plane,
>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  						   int pipe)
>  {
> -	struct intel_plane *cursor;
> -	struct intel_plane_state *state;
> +	struct intel_plane *cursor = NULL;
> +	struct intel_plane_state *state = NULL;
> +	int ret;
>  
>  	cursor = kzalloc(sizeof(*cursor), GFP_KERNEL);
> -	if (cursor == NULL)
> -		return NULL;
> +	if (!cursor)
> +		goto fail;
>  
>  	state = intel_create_plane_state(&cursor->base);
> -	if (!state) {
> -		kfree(cursor);
> -		return NULL;
> -	}
> +	if (!state)
> +		goto fail;
>  	cursor->base.state = &state->base;
>  
>  	cursor->can_scale = false;
> @@ -14166,11 +14172,13 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  	cursor->update_plane = intel_update_cursor_plane;
>  	cursor->disable_plane = intel_disable_cursor_plane;
>  
> -	drm_universal_plane_init(dev, &cursor->base, 0,
> -				 &intel_plane_funcs,
> -				 intel_cursor_formats,
> -				 ARRAY_SIZE(intel_cursor_formats),
> -				 DRM_PLANE_TYPE_CURSOR, NULL);
> +	ret = drm_universal_plane_init(dev, &cursor->base, 0,
> +				       &intel_plane_funcs,
> +				       intel_cursor_formats,
> +				       ARRAY_SIZE(intel_cursor_formats),
> +				       DRM_PLANE_TYPE_CURSOR, NULL);
> +	if (ret)
> +		goto fail;
>  
>  	if (INTEL_INFO(dev)->gen >= 4) {
>  		if (!dev->mode_config.rotation_property)
> @@ -14190,6 +14198,12 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  	drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs);
>  
>  	return &cursor->base;
> +
> +fail:
> +	kfree(state);
> +	kfree(cursor);
> +
> +	return NULL;
>  }
>  
>  static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 8821533..0f3e230 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1025,8 +1025,8 @@ static uint32_t skl_plane_formats[] = {
>  int
>  intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  {
> -	struct intel_plane *intel_plane;
> -	struct intel_plane_state *state;
> +	struct intel_plane *intel_plane = NULL;
> +	struct intel_plane_state *state = NULL;
>  	unsigned long possible_crtcs;
>  	const uint32_t *plane_formats;
>  	int num_plane_formats;
> @@ -1036,13 +1036,15 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  		return -ENODEV;
>  
>  	intel_plane = kzalloc(sizeof(*intel_plane), GFP_KERNEL);
> -	if (!intel_plane)
> -		return -ENOMEM;
> +	if (!intel_plane) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
>  
>  	state = intel_create_plane_state(&intel_plane->base);
>  	if (!state) {
> -		kfree(intel_plane);
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto fail;
>  	}
>  	intel_plane->base.state = &state->base;
>  
> @@ -1097,28 +1099,34 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
>  		break;
>  	default:
> -		kfree(intel_plane);
> -		return -ENODEV;
> +		MISSING_CASE(INTEL_INFO(dev)->gen);
> +		ret = -ENODEV;
> +		goto fail;
>  	}
>  
>  	intel_plane->pipe = pipe;
>  	intel_plane->plane = plane;
>  	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
>  	intel_plane->check_plane = intel_check_sprite_plane;
> +
>  	possible_crtcs = (1 << pipe);
> +
>  	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
>  				       &intel_plane_funcs,
>  				       plane_formats, num_plane_formats,
>  				       DRM_PLANE_TYPE_OVERLAY, NULL);
> -	if (ret) {
> -		kfree(intel_plane);
> -		goto out;
> -	}
> +	if (ret)
> +		goto fail;
>  
>  	intel_create_rotation_property(dev, intel_plane);
>  
>  	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
>  
> -out:
> +	return 0;
> +
> +fail:
> +	kfree(state);
> +	kfree(intel_plane);
> +
>  	return ret;
>  }
> -- 
> 2.4.3

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