Re: [PATCH 07/24] drm/i915: Remove useless checks from primary enable/disable

Paulo Zanoni
2014-04-07T20:04:23Z

2014-03-10 8:57 GMT-03:00 Daniel Vetter <daniel@ffwll.ch>:
> On Mon, Mar 10, 2014 at 12:20 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>> On Fri, Mar 07, 2014 at 10:29:26PM +0100, Daniel Vetter wrote:
>>> On Fri, Mar 07, 2014 at 06:32:14PM +0200, ville.syrjala@linux.intel.com wrote:
>>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> >
>>> > We won't be calling intel_enable_primary_plane() or
>>> > intel_disable_primary_plane() with the primary plane in the
>>> > wrong state. So remove the useless DISPLAY_PLANE_ENABLE checks.
>>> >
>>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Convert them to asserts, or are we already covered in this regard?
>>>
>>> /me has an uncanny love for self-checks ...
>>
>> There's a WARN() for the crtc->primary_enabled flag in there, which
>> should cover things. Although we could add some sanity check that makes
>> sure the DISPLAY_PLANE_ENABLE bit matches crtc->primary_enabled.
>
> Yeah, that's what I've thought of - we generally double-check hw state
> before frobbing it.

I agree with Daniel here, let's convert it to a WARN.

> -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

--
Paulo Zanoni