| Message ID | 20190920065626.3525-5-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DSB enablement. | expand |
On Fri, 20 Sep 2019, Animesh Manna <animesh.manna@intel.com> wrote: > DSB can program large set of data through indexed register write > (opcode 0x9) in one shot. DSB feature can be used for bulk register > programming e.g. gamma lut programming, HDR meta data programming. > > v1: initial version. > v2: simplified code by using ALIGN(). (Chris) > v3: ascii table added as code comment. (Shashank) > v4: cosmetic changes done. (Shashank) > v5: reset ins_start_offset. (Jani) > > Cc: Shashank Sharma <shashank.sharma@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dsb.c | 67 ++++++++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_dsb.h | 8 +++ > 2 files changed, 75 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c > index f94cd6dc98b6..0b5119135d4d 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.c > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > @@ -12,8 +12,10 @@ > /* DSB opcodes. */ > #define DSB_OPCODE_SHIFT 24 > #define DSB_OPCODE_MMIO_WRITE 0x1 > +#define DSB_OPCODE_INDEXED_WRITE 0x9 > #define DSB_BYTE_EN 0xF > #define DSB_BYTE_EN_SHIFT 20 > +#define DSB_REG_VALUE_MASK 0xfffff > > struct intel_dsb * > intel_dsb_get(struct intel_crtc *crtc) > @@ -83,9 +85,74 @@ void intel_dsb_put(struct intel_dsb *dsb) > mutex_unlock(&i915->drm.struct_mutex); > dsb->cmd_buf = NULL; > dsb->free_pos = 0; > + dsb->ins_start_offset = 0; This is not enough to address the sequence I described. Sure, we're not hitting the issue now, but it's not a benign thing. So imagine you do: intel_dsb_get() intel_dsb_indexed_reg_write(dsb, FOO, 0); intel_dsb_indexed_reg_write(dsb, FOO, 0); intel_dsb_reg_write(dsb, BAR, 0); intel_dsb_indexed_reg_write(dsb, FOO, 0); intel_dsb_commit() intel_dsb_put() Do you see what happens in the last indexed write? It looks at the *first* indexed writes for whether this is a continuation, updating the size there, ignoring the fact that there's something else in between. BR, Jani. > } > } > > +void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg, > + u32 val) > +{ > + struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb); > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + u32 *buf = dsb->cmd_buf; > + u32 reg_val; > + > + if (!buf) { > + I915_WRITE(reg, val); > + return; > + } > + > + if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) { > + DRM_DEBUG_KMS("DSB buffer overflow\n"); > + return; > + } > + > + /* > + * For example the buffer will look like below for 3 dwords for auto > + * increment register: > + * +--------------------------------------------------------+ > + * | size = 3 | offset &| value1 | value2 | value3 | zero | > + * | | opcode | | | | | > + * +--------------------------------------------------------+ > + * + + + + + + + > + * 0 4 8 12 16 20 24 > + * Byte > + * > + * As every instruction is 8 byte aligned the index of dsb instruction > + * will start always from even number while dealing with u32 array. If > + * we are writing odd no of dwords, Zeros will be added in the end for > + * padding. > + */ > + reg_val = buf[dsb->ins_start_offset + 1] & DSB_REG_VALUE_MASK; > + if (reg_val != i915_mmio_reg_offset(reg)) { > + /* Every instruction should be 8 byte aligned. */ > + dsb->free_pos = ALIGN(dsb->free_pos, 2); > + > + dsb->ins_start_offset = dsb->free_pos; > + > + /* Update the size. */ > + buf[dsb->free_pos++] = 1; > + > + /* Update the opcode and reg. */ > + buf[dsb->free_pos++] = (DSB_OPCODE_INDEXED_WRITE << > + DSB_OPCODE_SHIFT) | > + i915_mmio_reg_offset(reg); > + > + /* Update the value. */ > + buf[dsb->free_pos++] = val; > + } else { > + /* Update the new value. */ > + buf[dsb->free_pos++] = val; > + > + /* Update the size. */ > + buf[dsb->ins_start_offset]++; > + } > + > + /* if number of data words is odd, then the last dword should be 0.*/ > + if (dsb->free_pos & 0x1) > + buf[dsb->free_pos] = 0; > +} > + > void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val) > { > struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb); > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h > index 0686d67b34d5..d6ced4422814 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.h > +++ b/drivers/gpu/drm/i915/display/intel_dsb.h > @@ -30,11 +30,19 @@ struct intel_dsb { > * and help in calculating tail of command buffer. > */ > int free_pos; > + > + /* > + * ins_start_offset will help to store start address > + * of the dsb instuction of auto-increment register. > + */ > + u32 ins_start_offset; > }; > > struct intel_dsb * > intel_dsb_get(struct intel_crtc *crtc); > void intel_dsb_put(struct intel_dsb *dsb); > void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val); > +void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg, > + u32 val); > > #endif
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c index f94cd6dc98b6..0b5119135d4d 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.c +++ b/drivers/gpu/drm/i915/display/intel_dsb.c @@ -12,8 +12,10 @@ /* DSB opcodes. */ #define DSB_OPCODE_SHIFT 24 #define DSB_OPCODE_MMIO_WRITE 0x1 +#define DSB_OPCODE_INDEXED_WRITE 0x9 #define DSB_BYTE_EN 0xF #define DSB_BYTE_EN_SHIFT 20 +#define DSB_REG_VALUE_MASK 0xfffff struct intel_dsb * intel_dsb_get(struct intel_crtc *crtc) @@ -83,9 +85,74 @@ void intel_dsb_put(struct intel_dsb *dsb) mutex_unlock(&i915->drm.struct_mutex); dsb->cmd_buf = NULL; dsb->free_pos = 0; + dsb->ins_start_offset = 0; } } +void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg, + u32 val) +{ + struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + u32 *buf = dsb->cmd_buf; + u32 reg_val; + + if (!buf) { + I915_WRITE(reg, val); + return; + } + + if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) { + DRM_DEBUG_KMS("DSB buffer overflow\n"); + return; + } + + /* + * For example the buffer will look like below for 3 dwords for auto + * increment register: + * +--------------------------------------------------------+ + * | size = 3 | offset &| value1 | value2 | value3 | zero | + * | | opcode | | | | | + * +--------------------------------------------------------+ + * + + + + + + + + * 0 4 8 12 16 20 24 + * Byte + * + * As every instruction is 8 byte aligned the index of dsb instruction + * will start always from even number while dealing with u32 array. If + * we are writing odd no of dwords, Zeros will be added in the end for + * padding. + */ + reg_val = buf[dsb->ins_start_offset + 1] & DSB_REG_VALUE_MASK; + if (reg_val != i915_mmio_reg_offset(reg)) { + /* Every instruction should be 8 byte aligned. */ + dsb->free_pos = ALIGN(dsb->free_pos, 2); + + dsb->ins_start_offset = dsb->free_pos; + + /* Update the size. */ + buf[dsb->free_pos++] = 1; + + /* Update the opcode and reg. */ + buf[dsb->free_pos++] = (DSB_OPCODE_INDEXED_WRITE << + DSB_OPCODE_SHIFT) | + i915_mmio_reg_offset(reg); + + /* Update the value. */ + buf[dsb->free_pos++] = val; + } else { + /* Update the new value. */ + buf[dsb->free_pos++] = val; + + /* Update the size. */ + buf[dsb->ins_start_offset]++; + } + + /* if number of data words is odd, then the last dword should be 0.*/ + if (dsb->free_pos & 0x1) + buf[dsb->free_pos] = 0; +} + void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val) { struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb); diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h index 0686d67b34d5..d6ced4422814 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.h +++ b/drivers/gpu/drm/i915/display/intel_dsb.h @@ -30,11 +30,19 @@ struct intel_dsb { * and help in calculating tail of command buffer. */ int free_pos; + + /* + * ins_start_offset will help to store start address + * of the dsb instuction of auto-increment register. + */ + u32 ins_start_offset; }; struct intel_dsb * intel_dsb_get(struct intel_crtc *crtc); void intel_dsb_put(struct intel_dsb *dsb); void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val); +void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg, + u32 val); #endif