[Mesa-dev] i965: Batch emission refactoring

Francisco Jerez currojerez at riseup.net
Wed Apr 29 09:07:51 PDT 2015


"Pohjolainen, Topi" <topi.pohjolainen at intel.com> writes:

> On Wed, Apr 29, 2015 at 06:54:34PM +0300, Francisco Jerez wrote:
>> "Pohjolainen, Topi" <topi.pohjolainen at intel.com> writes:
>> 
>> > On Tue, Apr 28, 2015 at 03:07:35PM -0700, Kenneth Graunke wrote:
>> >> On Wednesday, April 22, 2015 11:47:20 PM Topi Pohjolainen wrote:
>> >> > Currently batch emission logic is bolted into using the current
>> >> > gl-state and currently bound user shader programs as input. This
>> >> > series refactors the api to allow caller to give individual bits of
>> >> > information needed explicitly instead of the emission logic
>> >> > deducing them from the current state.
>> >> > 
>> >> > This is needed to support blorp style gl-state-agnostic launching
>> >> > of internal utility shaders - shaders used for 2D blitting and
>> >> > buffer clearing/resolving.
>> >> > 
>> >> > I have a follow-up series ready that is actually leveraging this,
>> >> > this series is simple set of refactors. I didn't mean it to, but
>> >> > it actually fixes one pigit test on ILK due to the way formats
>> >> > are set for texture surfaces: arb_copy_image.arb_copy_image-formats.
>> >> > 
>> >> > Patches 6-13 all address texture surface setup. They move all the
>> >> > decision making of values into the hardware agnostic dispatcher
>> >> > leaving the hw-specific part just to deal with formatting.
>> >> > 
>> >> > Topi Pohjolainen (18):
>> >> >   i965: Refactor rb surface setup to allow caller to store offsets
>> >> >   i965: Expose and refactor brw_update_renderbuffer_surfaces()
>> >> >   i965: Refactor and expose brw_upload_binding_table()
>> >> >   i965: Remove dependency to tex object in default color setup
>> >> >   i965: Refactor sampler state setup
>> >> >   i965: Move texture buffer dispatch into single location
>> >> >   i965/gen8: Use miptree format in the surface setup
>> >> >   i965: Move tex miptree and format resolving into dispatcher
>> >> >   i965: Move texture swizzle resolving into dispatcher
>> >> >   i965: Pass integer format flag as parameter to surface setup
>> >> >   i965: Refactor effective depth calculation
>> >> >   i965: Pass texture target as parameter for surface setup
>> >> >   i965: Pass slice details as parameters for surface setup
>> >> 
>> >> I requested a small change on this patch.
>> >> 
>> >> >   i965/wm/gen6: Refactor program offset setup
>> >> 
>> >> I NAK'd this one.
>> >> 
>> >> The rest of this 18 patch series looks great to me and is:
>> >> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>> >> 
>> >> It looks like Curro landed different texture surface state refactoring
>> >> patches in the meantime, though...so the two of you will need to decide
>> >> how to sort that out :(
>> >
>> > It looks like I need to do more rebasing than this. There are now these
>> > two in master:
>> >
>> > e17dc00 i965/gen8: Factor out texture surface state set-up from gen8_update_text
>> > 6f26ffa i965/gen7: Factor out texture surface state set-up from gen7_update_text
>> >
>> > Curro, neither has any review tags, why is this?
>> 
>> Ahh, sorry for the collision.  I also landed "i965: Add helper functions
>> to calculate the slice pitch of an array or 3D miptree." without
>> apparent review at the same time.  I proposed both the texture and
>> miptree refactor so long ago (in December 2013 IIRC) that I had kind of
>> lost hope of it getting reviewed anytime soon, and they were a real mess
>> to rebase (I actually introduced regressions accidentally during my
>> regular rebases due to the constant changes in those code paths).  I
>> warned several days before I pushed them and nobody seemed to care.
>> 
>> In fact I got some R-b tags from Paul Berry at some point, but by the
>> time I decided to push the patches things had changed so much that it
>> didn't seem fair anymore to include his tags...
>> 
>> Which of your patches do they conflict with?  If they happened to change
>> the API in a different direction that you were meaning to, maybe we can
>> have a chat about it tomorrow?
>
> Well, your changes would have been easy to put on top of mine. But I have
> seven patch incremental refactoring of which none apply anymore.

Sorry, if you need to make non-trivial changes requiring additional
review don't hesitate to ping me.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150429/08972418/attachment.sig>


More information about the mesa-dev mailing list