[PATCH] VGA arbiter: No need for arbitration around CreateGC.

Jamey Sharp jamey at minilop.net
Wed Jul 14 07:25:39 PDT 2010


Thanks for reviewing! (And thanks to Alex and Jeremy for this round of
reviews too!)

I don't quite understand your comments on this patch, though, Tiago.

On Wed, Jul 14, 2010 at 6:25 AM, Vignatti Tiago (Nokia-MS/Helsinki)
<tiago.vignatti at nokia.com> wrote:
> On Wed, Jul 14, 2010 at 03:19:11PM +0200, Vignatti Tiago (Nokia-MS/Helsinki) wrote:
>> as discussed already in private:
>>
>>     Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>

First, I don't understand this use of signed-off-by. Did you mean reviewed-by?

> oops, I forgot to mention that you can remove the entire function wrapping
> there, Jamey. Just remove VGAarbiterCreateGC entirely.

That's why I want this patch applied, but I believe the rest of the
patches on my for-1.10 branch are needed before it's safe to entirely
delete this function, and I haven't sent those out for review yet.

Currently, as far as I can tell, the VGA arbiter needs a wrapper
around ValidateGC, just so that it can unwrap the ops on the way down
the ValidateGC chain, in case some other ValidateGC wrapper changes
the ops pointer. And the current way to get a wrapper around
ValidateGC is to hook the funcs chain from CreateGC.

When my later patches impose the rule that GC funcs must not modify
the GC ops, then it's safe to delete an awful lot of code, and even
more when I then move the funcs and ops chains to the ScreenRec.

> Also, if all GC operations don't touch the hw at all for all kind of accel,
> then we can extend this patch for the other functions there.

I don't think anybody's GC funcs touch the hardware, which is good
since the VGA arbiter is already not taking the arbitration lock
around those. But hopefully somebody's GC ops are touching the
hardware or else the whole ops abstraction is pointless.

With those clarifications, are you satisfied with this patch?

Jamey


More information about the xorg-devel mailing list