[Mesa-dev] [PATCH 02/13] i965: Allow passing target_bo=NULL to brw_emit_reloc()

[Kenneth Graunke]

On Thursday, July 20, 2017 7:29:52 AM PDT Chris Wilson wrote:
> Quoting Chris Wilson (2017-07-20 15:15:02)
> > Quoting Kenneth Graunke (2017-07-19 21:08:23)
> > > On Wednesday, July 19, 2017 3:09:10 AM PDT Chris Wilson wrote:
> > > > Sometimes we want to emit a relocation to a NULL surface when the
> > > > constructing the batch. If we push the NULL handling into the common
> > > > brw_emit_reloc() we can make the batch construction itself more
> > > > readable.
> > > 
> > > I don't like this...
> > > 
> > > There is no such thing as a "relocation to a NULL surface".  No relocation
> > > is emittted in this case.  It either means the field is relative to a base
> > > address, and is simply an offset, or the address is unused and we're setting
> > > a NULL pointer combined with other bits packed into the same DWord.
> > 
> > There's actually no such thing as a relocation ;) I only put it out
> > because there were so many duplicated checks.
> 
> The difference is that in the always use brw_emit_reloc patch end up
> with 
> 
> @@ -661,18 +662,17 @@ brw_emit_buffer_surface_state(struct brw_context *brw,
>                                    out_offset);
>  
>     isl_buffer_fill_state(&brw->isl_dev, dw,
> -                         .address = (bo ? bo->offset64 : 0) + buffer_offset,
> +                         .address = (bo ?
> +                                     brw_emit_reloc(&brw->batch,
> +                                                    *out_offset + brw->isl_dev.ss.addr_offset,
> +                                                    bo, buffer_offset,
> +                                                    I915_GEM_DOMAIN_SAMPLER,
> +                                                    (rw ? I915_GEM_DOMAIN_SAMPLER : 0)) :
> +                                     buffer_offset),

Not that much of an eyesore if you do...

                            .address = !bo ? buffer_offset :
                                       brw_emit_reloc(&brw->batch,
                                                      *out_offset + brw->isl_dev.ss.addr_offset,
                                                      bo, buffer_offset,
                                                      I915_GEM_DOMAIN_SAMPLER,
                                                      (rw ? I915_GEM_DOMAIN_SAMPLER : 0),

one extra line at the top, no extra parens or else case hanging off the end...

>                           .size = buffer_size,
>                           .format = surface_format,
>                           .stride = pitch,
>                           .mocs = tex_mocs[brw->gen]);
> 
> which was a bit too much of an eyesore for me.
> -Chris
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170720/ba288d75/attachment.sig>