On Tue, Sep 02, 2014 at 10:45:45AM +0100, Damien Lespiau wrote:
> On Tue, Sep 02, 2014 at 10:14:17AM +0100, Arun Siluvery wrote:
> > This rework is based on suggestion from Chris.
> > Now w/a are organized in an array and all of them are emitted in
> > single fn instead of sending them individually. This approach is
> > clean and new w/a can be added with minimal changes.
> > The same array can be used when exporting them to debugfs and the
> > temporary array in the current implementation is not required.
> >
> > v2: As suggested by Damien a new w/a reg definition is added.
> > This helps in initializing w/a that are unmasked registers.
> >
> > Signed-off-by: Arun Siluvery <arun.siluvery@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 158 ++++++++++++++------------------
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 26 ++++++
> > 2 files changed, 93 insertions(+), 91 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index c5e4dc7..3ed0ad5 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -650,87 +650,80 @@ err:
> > return ret;
> > }
> >
> > -static inline void intel_ring_emit_wa(struct intel_engine_cs *ring,
> > - u32 addr, u32 value)
> > +static int i915_request_emit_lri(struct intel_engine_cs *ring,
> > + int num_registers,
> > + const struct intel_wa_reg *lri_list)
> > {
>
> [...]
>
> > + for (i = 0; i < num_registers; ++i) {
> > + u32 value;
> > + const struct intel_wa_reg *p = (lri_list + i);
> >
> > + if (p->masked_register)
> > + value = (p->mask << 16) | p->value;
> > + else
> > + value = (I915_READ(p->addr) & ~p->mask) | p->value;
>
> There's one case when this won't work, when several WAs for a single
> 'normal' register are defined. The read done here means only the last of
> those W/As will end up being applied (because the last LRI to that
> register will be the value that ends up in the register. We'll probably
> need to coalesce all W/A defined for a single normal register into one
> write.
>
> Considering that, and the comment about the define, maybe the simplest
> way to go forward with the patch is to forget the non-masked case for
> the moment, especially as it's not used in this patch.
If you are still considering an in-kernel table, despite that it can be
done in userspace, design it to track all w/a and not just the ring
specific fixups.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
