| Message ID | 20190821063236.19705-11-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DSB enablement. | expand |
Quoting Animesh Manna (2019-08-21 07:32:30) > Batch buffer will be created through dsb-reg-write function which can have > single/multiple request based on usecase and once the buffer is ready > commit function will trigger the execution of the batch buffer. All > the registers will be updated simultaneously. > > Cc: Imre Deak <imre.deak@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dsb.c | 43 ++++++++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_dsb.h | 1 + > 2 files changed, 44 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c > index f97d0c06a049..7e0a9b37f702 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.c > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > @@ -191,3 +191,46 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val) > DSB_OPCODE_SHIFT) | DSB_BYTE_EN | > i915_mmio_reg_offset(reg); > } > + > +void intel_dsb_commit(struct intel_dsb *dsb) > +{ > + struct intel_crtc *crtc = dsb->crtc; > + struct drm_device *dev = crtc->base.dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + enum pipe pipe = crtc->pipe; > + u32 cmd_buf_tail, cmd_buf_size; > + > + if (!dsb->free_pos) > + return; > + > + if (!intel_dsb_enable_engine(dsb)) > + goto reset; > + > + if (is_dsb_busy(dsb)) { > + DRM_DEBUG_KMS("HEAD_PTR write failed - dsb engine is busy.\n"); > + goto reset; > + } > + I915_WRITE(DSB_HEAD_PTR(pipe, dsb->id), dsb->cmd_buf_head); > + > + cmd_buf_size = dsb->free_pos * 4; > + cmd_buf_tail = round_up((dsb->cmd_buf_head + cmd_buf_size), > + CACHELINE_BYTES); head is already page-aligned. tail = ALIGN(dst->free_pos * 4, CACHELINE_BYTES); I915_WRITE(DSB_TAIL(pipe, dsb->id), i915_ggtt_offset(dsb->vma) + tail); > + > + if (is_dsb_busy(dsb)) { > + DRM_DEBUG_KMS("TAIL_PTR write failed - dsb engine is busy.\n"); > + goto reset; > + } > + DRM_DEBUG_KMS("DSB execution started - buf-size %u, head 0x%x," > + "tail 0x%x\n", cmd_buf_size, dsb->cmd_buf_head, > + cmd_buf_tail); > + I915_WRITE(DSB_TAIL_PTR(pipe, dsb->id), cmd_buf_tail); This looks very suspect. You enable the HW before setting up the cmdbuf, so what is executing? Is the execution latched on TAIL or the CTL? Is it latched at all? > + if (wait_for(!is_dsb_busy(dsb), 1)) { > + DRM_ERROR("Timed out waiting for DSB workload completion.\n"); > + goto reset; > + } > + > +reset: > + memset(dsb->cmd_buf, 0, DSB_BUF_SIZE); Again, why? -Chris
Hi, On 8/22/2019 12:13 AM, Chris Wilson wrote: > Quoting Animesh Manna (2019-08-21 07:32:30) >> Batch buffer will be created through dsb-reg-write function which can have >> single/multiple request based on usecase and once the buffer is ready >> commit function will trigger the execution of the batch buffer. All >> the registers will be updated simultaneously. >> >> Cc: Imre Deak <imre.deak@intel.com> >> Cc: Jani Nikula <jani.nikula@intel.com> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dsb.c | 43 ++++++++++++++++++++++++ >> drivers/gpu/drm/i915/display/intel_dsb.h | 1 + >> 2 files changed, 44 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c >> index f97d0c06a049..7e0a9b37f702 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dsb.c >> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c >> @@ -191,3 +191,46 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val) >> DSB_OPCODE_SHIFT) | DSB_BYTE_EN | >> i915_mmio_reg_offset(reg); >> } >> + >> +void intel_dsb_commit(struct intel_dsb *dsb) >> +{ >> + struct intel_crtc *crtc = dsb->crtc; >> + struct drm_device *dev = crtc->base.dev; >> + struct drm_i915_private *dev_priv = to_i915(dev); >> + enum pipe pipe = crtc->pipe; >> + u32 cmd_buf_tail, cmd_buf_size; >> + >> + if (!dsb->free_pos) >> + return; >> + >> + if (!intel_dsb_enable_engine(dsb)) >> + goto reset; >> + >> + if (is_dsb_busy(dsb)) { >> + DRM_DEBUG_KMS("HEAD_PTR write failed - dsb engine is busy.\n"); >> + goto reset; >> + } >> + I915_WRITE(DSB_HEAD_PTR(pipe, dsb->id), dsb->cmd_buf_head); >> + >> + cmd_buf_size = dsb->free_pos * 4; >> + cmd_buf_tail = round_up((dsb->cmd_buf_head + cmd_buf_size), >> + CACHELINE_BYTES); > head is already page-aligned. > > tail = ALIGN(dst->free_pos * 4, CACHELINE_BYTES); > I915_WRITE(DSB_TAIL(pipe, dsb->id), i915_ggtt_offset(dsb->vma) + tail); Ok. > >> + >> + if (is_dsb_busy(dsb)) { >> + DRM_DEBUG_KMS("TAIL_PTR write failed - dsb engine is busy.\n"); >> + goto reset; >> + } >> + DRM_DEBUG_KMS("DSB execution started - buf-size %u, head 0x%x," >> + "tail 0x%x\n", cmd_buf_size, dsb->cmd_buf_head, >> + cmd_buf_tail); >> + I915_WRITE(DSB_TAIL_PTR(pipe, dsb->id), cmd_buf_tail); > This looks very suspect. You enable the HW before setting up the cmdbuf, > so what is executing? Is the execution latched on TAIL or the CTL? Is it > latched at all? Yes, execution is latched with TAIL - it the trigger to start dsb execution. > >> + if (wait_for(!is_dsb_busy(dsb), 1)) { >> + DRM_ERROR("Timed out waiting for DSB workload completion.\n"); >> + goto reset; >> + } >> + >> +reset: >> + memset(dsb->cmd_buf, 0, DSB_BUF_SIZE); > Again, why? Can be optimized by filling zero in the unused space before DSB_TAIL_PTR. Will do. Currently have single commit between get and put call. For multiple commit we can use same buffer, so want to clear the buffer so that can be used for next commit call. Regards, Animesh > -Chris
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c index f97d0c06a049..7e0a9b37f702 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.c +++ b/drivers/gpu/drm/i915/display/intel_dsb.c @@ -191,3 +191,46 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val) DSB_OPCODE_SHIFT) | DSB_BYTE_EN | i915_mmio_reg_offset(reg); } + +void intel_dsb_commit(struct intel_dsb *dsb) +{ + struct intel_crtc *crtc = dsb->crtc; + struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = to_i915(dev); + enum pipe pipe = crtc->pipe; + u32 cmd_buf_tail, cmd_buf_size; + + if (!dsb->free_pos) + return; + + if (!intel_dsb_enable_engine(dsb)) + goto reset; + + if (is_dsb_busy(dsb)) { + DRM_DEBUG_KMS("HEAD_PTR write failed - dsb engine is busy.\n"); + goto reset; + } + I915_WRITE(DSB_HEAD_PTR(pipe, dsb->id), dsb->cmd_buf_head); + + cmd_buf_size = dsb->free_pos * 4; + cmd_buf_tail = round_up((dsb->cmd_buf_head + cmd_buf_size), + CACHELINE_BYTES); + + if (is_dsb_busy(dsb)) { + DRM_DEBUG_KMS("TAIL_PTR write failed - dsb engine is busy.\n"); + goto reset; + } + DRM_DEBUG_KMS("DSB execution started - buf-size %u, head 0x%x," + "tail 0x%x\n", cmd_buf_size, dsb->cmd_buf_head, + cmd_buf_tail); + I915_WRITE(DSB_TAIL_PTR(pipe, dsb->id), cmd_buf_tail); + if (wait_for(!is_dsb_busy(dsb), 1)) { + DRM_ERROR("Timed out waiting for DSB workload completion.\n"); + goto reset; + } + +reset: + memset(dsb->cmd_buf, 0, DSB_BUF_SIZE); + dsb->free_pos = 0; + intel_dsb_disable_engine(dsb); +} diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h index 1fa893cc8c2e..7330add3c96f 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.h +++ b/drivers/gpu/drm/i915/display/intel_dsb.h @@ -42,5 +42,6 @@ struct intel_dsb { struct intel_dsb * intel_dsb_get(struct intel_crtc *crtc); void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val); +void intel_dsb_commit(struct intel_dsb *dsb); #endif
Batch buffer will be created through dsb-reg-write function which can have single/multiple request based on usecase and once the buffer is ready commit function will trigger the execution of the batch buffer. All the registers will be updated simultaneously. Cc: Imre Deak <imre.deak@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_dsb.c | 43 ++++++++++++++++++++++++ drivers/gpu/drm/i915/display/intel_dsb.h | 1 + 2 files changed, 44 insertions(+)