Message ID | 20190918075745.17076-5-animesh.manna@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | DSB enablement. | expand |
On Wed, 18 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) > > 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 | 66 ++++++++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_dsb.h | 8 +++ > 2 files changed, 74 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c > index f94cd6dc98b6..2d80e452e284 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) > @@ -86,6 +88,70 @@ void intel_dsb_put(struct intel_dsb *dsb) > } > } > > +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; dsb->ins_start_offset is never reset anywhere. Imagine you do: 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); The last one will screw up the previous indexed writes. BR, Jani. > + 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
On 9/19/2019 10:08 PM, Jani Nikula wrote: > On Wed, 18 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) >> >> 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 | 66 ++++++++++++++++++++++++ >> drivers/gpu/drm/i915/display/intel_dsb.h | 8 +++ >> 2 files changed, 74 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c >> index f94cd6dc98b6..2d80e452e284 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) >> @@ -86,6 +88,70 @@ void intel_dsb_put(struct intel_dsb *dsb) >> } >> } >> >> +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; > dsb->ins_start_offset is never reset anywhere. > > Imagine you do: > > 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); > > The last one will screw up the previous indexed writes. > > BR, > Jani. > > >> + 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; >> + Not sure if I got your point correctly, but here is the ins_start_offset getting reset, with the new free_pos aligned with 2 - Shashank >> + /* 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..2d80e452e284 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) @@ -86,6 +88,70 @@ void intel_dsb_put(struct intel_dsb *dsb) } } +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