Re: [PATCH 08/24] drm/i915: Shuffle wait_for_vblank out of primary_enable/disable funcs

Daniel Vetter
2014-04-29T14:00:57Z

On Tue, Apr 08, 2014 at 09:55:45PM +0300, Ville Syrjälä wrote:
> On Mon, Apr 07, 2014 at 05:27:41PM -0300, Paulo Zanoni wrote:
> > 2014-03-07 13:32 GMT-03:00 <ville.syrjala@linux.intel.com>:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Rather than have a wait_for_vblank() in the primary plane enable/disable
> > > funcs, move the wait_for_vblank() to happen after enabling/disabling all
> > > planes.
> >
> > Why exactly? What is improved? Are we solving a bug? What are the
> > risks? What's the problem with the current code? Did you check the
> > modeset sequence documentation of every single platform (since you
> > changed them all) to make sure this is safe?
>
> Just another step towards getting all the planes enabled/disabled
> atomically during a modeset. I should have probably yanked it from
> this series since it shouldn't be strictly needed for the watermark
> stuff. At least I can't think of a reason now. I guess I included
> it here just to make things a bit more difficult for the new watermark
> update mechanism.
>
> In any case there's nothing magical about the primary plane, so we
> shouldn't treat it as such. That's my excuse for this patch anyway.
>
> > Please update the commit message with the answers.
> >
> > Also, we should probably update the first comment of hsw_enable_ips.
> > It seems things have changed since it was written.
>
> I thought I had. Ah no, that's part of the mmio vs. cs flip race series.
> Looks like the wait for vblank changes there will conflict with this
> stuff anyway. I'll have to revisit that series, and hopefully get it
> merged before this series so that we can ignore this patch entirely.

I'll ignore this one her for now ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch