Re: [PATCH 3/4] drm/i915: Bail if plane/crtc init fails

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

On Fri, Nov 04, 2016 at 08:48:21PM +0000, Chris Wilson wrote:
> On Tue, Oct 25, 2016 at 06:58:02PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Due to the plane->index not getting readjusted in drm_plane_cleanup(),
> > we can't continue initialization of some plane/crtc init fails.
> > Well, we sort of could I suppose if we left all initialized planes on
> > the list, but that would expose those planes to userspace as well.
> > 
> > But for crtcs the situation is even worse since we assume that
> > pipe==crtc index occasionally, so we can't really deal with a partially
> > initialize set of crtcs.
> > 
> > So seems safest to just abort the entire thing if anything goes wrong.
> > All the failure paths here are kmalloc()s anyway, so it seems unlikely
> > we'd get very far if these start failing.
> 
> smatch spotted ERR_PTR(0)
> 
> > @@ -15296,22 +15304,30 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> >  	}
> >  
> >  	primary = intel_primary_plane_create(dev, pipe);
> > -	if (!primary)
> > +	if (IS_ERR(primary)) {
> > +		ret = PTR_ERR(primary);
> 
> Here...

This looks correct to me, but the cursor and sprite paths are clearly
crap.

> 
> >  		goto fail;
> > +	}
> >  
> >  	for_each_sprite(dev_priv, pipe, sprite) {
> > -		ret = intel_plane_init(dev, pipe, sprite);
> > -		if (ret)
> > -			DRM_DEBUG_KMS("pipe %c sprite %c init failed: %d\n",
> > -				      pipe_name(pipe), sprite_name(pipe, sprite), ret);
> > +		struct intel_plane *plane;
> > +
> > +		plane = intel_sprite_plane_create(dev, pipe, sprite);
> > +		if (!plane) {
> > +			ret = PTR_ERR(plane);
> 
> and here.
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

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