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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.