From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 00/12] drm/i915: Some cleanups and fixes to the sprite code v2 Date: Mon, 7 Oct 2013 11:04:37 +0200 Message-ID: <20131007090437.GY31334@phenom.ffwll.local> References: <1380639741-5276-1-git-send-email-ville.syrjala@linux.intel.com> <20131004102816.GC31587@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f180.google.com (mail-ea0-f180.google.com [209.85.215.180]) by gabe.freedesktop.org (Postfix) with ESMTP id D970AE7136 for ; Mon, 7 Oct 2013 02:04:18 -0700 (PDT) Received: by mail-ea0-f180.google.com with SMTP id h10so3094507eaj.11 for ; Mon, 07 Oct 2013 02:04:18 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20131004102816.GC31587@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson , ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Oct 04, 2013 at 11:28:16AM +0100, Chris Wilson wrote: > On Tue, Oct 01, 2013 at 06:02:09PM +0300, ville.syrjala@linux.intel.com wrote: > > Chris asked for some renames and assertions during v1. While adding those I > > noticed that what I did in the original patch 02 didn't match quite so well > > with the assertions. So I modified patch 02 a bit, and that caused quite a bit > > of bit of rebase issues for most of the other patches, so I figured it's better > > to repost the whole thing. > > > > Changes from v1: > > - Move the primary disable/enable calls inside intel_crtc->active checks > > in intel_update_plane/intel_disable_plane. That also ate up patch 03 from > > the original series. > > - Add primary_disabled WARNs > > - Rename primary plane funcs > > - Flush primary plane changes from sprite code > > - Add a POSTING_READ() to intel_flush_primary_plane. This shouldn't really > > be necessary now that I think about it some more. So we might want to drop > > that change... > > Looks good, very good, to me. > > Even with throwing up over FBC, > > Reviewed-by: Chris Wilson > > except for > > 08/12: drm/i915: Enable/disable IPS when primary is > enabled/disabled > > For which the code looks ok, but only merits an > Acked-by: Chris Wilson All merged, thanks for patches&review. Now my problem here is that I'm really uneasy with our complete lack of testcoverage for all these corner-cases. I know that we can't really get full functional testing going for sprites/planes/cursors before we have the CRC stuff all merged, but at least exercising all this code would be great. So can we please move "polish/create testcases for cursor/sprite/plane corner-cases and push the to igt" to the top-spot for all things planes? At least before we start to wreak utter havoc with atomic pageflips/modeset I want to have some assurance that we don't break all the low-level functions we've painstakingly beaten into shape in the past few months ... Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch