Re: [PATCH 9/9] drm/i915: Make all plane disables use 'update_plane'

From: Daniel Vetter <daniel@ffwll.ch>
To: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 9/9] drm/i915: Make all plane disables use 'update_plane'
Date: Mon, 1 Dec 2014 17:46:53 +0100	[thread overview]
Message-ID: <20141201164653.GD32117@phenom.ffwll.local> (raw)
In-Reply-To: <547C7562.9010004@gmail.com>

On Mon, Dec 01, 2014 at 04:04:18PM +0200, Ander Conselvan de Oliveira wrote:
> On 11/24/2014 09:53 PM, Matt Roper wrote:
> >+/**
> >+ * intel_disable_plane - disable a plane
> >+ * @plane: plane to disable
> >+ *
> >+ * General disable handler for all plane types.
> >+ */
> >+int
> >+intel_disable_plane(struct drm_plane *plane)
> >+{
> >+	if (!plane->fb)
> >+		return 0;
> >+
> >+	BUG_ON(!plane->crtc);
> 
> WARN_ON? Maybe not since intel_update_plane will oops. I'm not sure what's
> the guideline here.

I'd go with a if (WARN_ON) return -EINVAL; or so. We've had a bunch of fun
cases when we've done the original hw state readout code where code
fizzled out on an Oops because the fb wasn't reconstructed. And doing that
at boot-up while holding console_lock is really bad for debugging.

But yeah in general keeping the BUG_ON if you'll oops otherwise anyway is
the correct thing to do.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx