From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org
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 [thread overview]
Message-ID: <20131007090437.GY31334@phenom.ffwll.local> (raw)
In-Reply-To: <20131004102816.GC31587@nuc-i3427.alporthouse.com>
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 <chris@chris-wilson.co.uk>
>
> 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 <chris@chris-wilson.co.uk>
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
prev parent reply other threads:[~2013-10-07 9:04 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-01 15:02 [PATCH 00/12] drm/i915: Some cleanups and fixes to the sprite code v2 ville.syrjala
2013-10-01 15:02 ` [PATCH 01/12] drm/i915: Set primary_disabled in intel_{enable, disable}_plane ville.syrjala
2013-10-01 15:02 ` [PATCH v2 02/12] drm/i915: Allow sprites to be configured on a disabled pipe ville.syrjala
2013-10-01 15:02 ` [PATCH 03/12] drm/i915: Reduce the time we hold struct mutex in sprite update_plane code ville.syrjala
2013-10-04 10:21 ` Chris Wilson
2013-10-04 10:40 ` Ville Syrjälä
2013-10-04 12:41 ` Daniel Vetter
2013-10-01 15:02 ` [PATCH 04/12] drm/i915: Kill a goto from sprite disable code ville.syrjala
2013-10-01 15:02 ` [PATCH 05/12] drm/i915: Do a bit of cleanup in the sprite code ville.syrjala
2013-10-01 15:02 ` [PATCH 06/12] drm/i915: Save user requested plane coordinates only on success ville.syrjala
2013-10-01 15:02 ` [PATCH 07/12] drm/i915: Do the fbc vs. primary plane enable/disable in the right order ville.syrjala
2013-10-04 10:24 ` Chris Wilson
2013-10-01 15:02 ` [PATCH 08/12] drm/i915: Enable/disable IPS when primary is enabled/disabled ville.syrjala
2013-10-01 15:02 ` [PATCH 09/12] drm/i915: Rename intel_flush_display_plane to intel_flush_primary_plane ville.syrjala
2013-10-01 15:02 ` [PATCH 10/12] drm/i915: Rename intel_{enable, disable}_plane to intel_{enable, disable}_primary_plane ville.syrjala
2013-10-01 15:02 ` [PATCH 11/12] drm/i915: WARN if primary plane state doesn't match expectations ville.syrjala
2013-10-01 15:02 ` [PATCH 12/12] drm/i915: Flush primary plane changes in sprite code ville.syrjala
2013-10-04 10:28 ` [PATCH 00/12] drm/i915: Some cleanups and fixes to the sprite code v2 Chris Wilson
2013-10-07 9:04 ` Daniel Vetter [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131007090437.GY31334@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox