Em Ter, 2015-10-20 às 17:03 +0100, Chris Wilson escreveu: > On Tue, Oct 20, 2015 at 11:49:51AM -0200, Paulo Zanoni wrote: > > This thing where we need to get the crtc either from the work > > structure or the fbc structure itself is confusing and unnecessary. > > Set fbc.crtc right when scheduling the enable work so we can always > > use it. > > Pardon? It was confusing to have the crtc passed along with the work > item as opposed to digging it out from an indirect link on the > dev_priv. > iirc work->crtc was part of the serialisation of the work item. > > What I think you meant was that you were passing around the wrong > struct, e.g. > intel_fbc_enable() just wants the crtc, in intel_fbc_flip_prepare() > you > check for the work struct, and then you should just use the work > struct. The problem is not what gets passed and how to retrieve it. The problem is that when we're in the other parts of the code we always have to keep in mind that if FBC is already enabled we have to get the CRTC from place A, if FBC is scheduled we have to get the CRTC from place B, and if it's disabled there's no CRTC. Having a single place to retrieve the CRTC from allows us to treat the "is enabled" and "is scheduled" cases as the same case, reducing the mistake surface. I guess I should add this to the commit message. Anyway, it's worth mentioning that later on this series when we introduce enable() and disable() we already set dev_priv->fbc.crtc at enable(), so all the activate/deactivate/update code can just look at that regardless of the current state. So this patch is just an intermediate step to keep everything simple and working until the enable/disable patch. > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
