[Intel-gfx] [PATCH 5/8] drm/i915: simplify FBC start/stop at invalidate/flush

Chris Wilson chris at chris-wilson.co.uk
Wed Jul 1 07:04:26 PDT 2015


On Tue, Jun 30, 2015 at 06:12:59PM -0300, Paulo Zanoni wrote:
> 2015-06-30 11:34 GMT-03:00 Chris Wilson <chris at chris-wilson.co.uk>:
> > I presume that start/stop are the highest, and control the sw state. And
> > that enable/disable are just hw interaction. And who sets fbc.enabled?
> > start()? enable()? disable()? stop()?
> >
> > In confusion,
> 
> I understand your concerns and I agree with you that this is
> confusing. I also agree that the addition of stop() makes things even
> worse. One of the problems is that intel_fbc_update() does
> "everything": it picks the CRTC, it can enable FBC, it can disable
> FBC, it can change the CRTC, etc. So we have: update(), enable(),
> disable(), flush() and invalidate(), and the patch added stop().
> 
> I had some patches that would move us to enable/disable (high level)
> activate/deactivate (low level), flush/invalidate (wrappers for
> activate/deactivate) and update (highest level). This would make the
> naming scheme similar to PSR. I wanted to merge the locking fixes
> first, but I can put everything on the same series if you want. Or
> leave this patch out of the "locking" series and add it to the next
> series...

To keep it minimal, a quick outline comment telling me the layers and
ordering would be of use right now to review the patches, and longer
term to review the code.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the Intel-gfx mailing list