On Thu, 12 Sep 2019, "Sharma, Shashank" <shashank.sharma@xxxxxxxxx> wrote:
> On 9/12/2019 12:41 AM, Animesh Manna wrote:
>> 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.
>>
>> v1: Initial version.
>> v2: Optimized code few places. (Chris)
>> v3: USed DRM_ERROR for dsb head/tail programming failure. (Shashank)
>>
>> Cc: Imre Deak <imre.deak@xxxxxxxxx>
>> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
>> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>> Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx>
>> Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/display/intel_dsb.c | 42 ++++++++++++++++++++++++
>> drivers/gpu/drm/i915/display/intel_dsb.h | 1 +
>> drivers/gpu/drm/i915/i915_reg.h | 2 ++
>> 3 files changed, 45 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
>> index 2b0ffc0afb74..eea86afb0583 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -212,3 +212,45 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
>> (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
>> i915_mmio_reg_offset(reg);
>> }
>> +
>> +void intel_dsb_commit(struct intel_dsb *dsb)
>> +{
>> + struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
>> + struct drm_device *dev = crtc->base.dev;
>> + struct drm_i915_private *dev_priv = to_i915(dev);
>> + enum pipe pipe = crtc->pipe;
>> + u32 tail;
>> +
>> + if (!dsb->free_pos)
>> + return;
>> +
>> + if (!intel_dsb_enable_engine(dsb))
>> + goto reset;
>> +
>> + if (is_dsb_busy(dsb)) {
>> + DRM_ERROR("HEAD_PTR write failed - dsb engine is busy.\n");
>> + goto reset;
>> + }
>> + I915_WRITE(DSB_HEAD(pipe, dsb->id), i915_ggtt_offset(dsb->vma));
>> +
>> + tail = ALIGN(dsb->free_pos * 4, CACHELINE_BYTES);
>> + if (tail > dsb->free_pos * 4)
>> + memset(&dsb->cmd_buf[dsb->free_pos], 0,
>> + (tail - dsb->free_pos * 4));
>> +
>> + if (is_dsb_busy(dsb)) {
>> + DRM_ERROR("TAIL_PTR write failed - dsb engine is busy.\n");
>> + goto reset;
>> + }
>> + DRM_DEBUG_KMS("DSB execution started - head 0x%x, tail 0x%x\n",
>> + i915_ggtt_offset(dsb->vma), tail);
>> + I915_WRITE(DSB_TAIL(pipe, dsb->id), i915_ggtt_offset(dsb->vma) + tail);
>> + if (wait_for(!is_dsb_busy(dsb), 1)) {
>> + DRM_ERROR("Timed out waiting for DSB workload completion.\n");
>> + goto reset;
>> + }
>> +
>> +reset:
>> + dsb->free_pos = 0;
>> + intel_dsb_disable_engine(dsb);
>
> I am still not very convince if a commit function should be void, I
> would still want it to return success or failure, so that we would know
> if the last operation was successful or not.
>
> I would wait Jani N to comment here, on what he feels about this.
The question becomes, what do you *do* with the return value? If none of
the callers are going to use it, don't return it.
BR,
Jani.
>
> - Shashank
>
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
>> index 9b2522f20bfb..7389c8c5b665 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
>> @@ -43,5 +43,6 @@ 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);
>> +void intel_dsb_commit(struct intel_dsb *dsb);
>>
>> #endif
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 2dbaa49f5c74..c77b5066d8dd 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -11687,6 +11687,8 @@ enum skl_power_gate {
>> #define _DSBSL_INSTANCE_BASE 0x70B00
>> #define DSBSL_INSTANCE(pipe, id) (_DSBSL_INSTANCE_BASE + \
>> (pipe) * 0x1000 + (id) * 100)
>> +#define DSB_HEAD(pipe, id) _MMIO(DSBSL_INSTANCE(pipe, id) + 0x0)
>> +#define DSB_TAIL(pipe, id) _MMIO(DSBSL_INSTANCE(pipe, id) + 0x4)
>> #define DSB_CTRL(pipe, id) _MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
>> #define DSB_ENABLE (1 << 31)
>> #define DSB_STATUS (1 << 0)
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
