[Intel-gfx] [PATCH v9 04/10] drm/i915/dsb: Indexed register write function for DSB.
Animesh Manna
animesh.manna at intel.com
Fri Sep 20 15:58:57 UTC 2019
On 9/20/2019 5:48 PM, Jani Nikula wrote:
> On Fri, 20 Sep 2019, Animesh Manna <animesh.manna at 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)
>> v6: update ins_start_offset in inel_dsb_reg_write.
>>
>> Cc: Shashank Sharma <shashank.sharma at intel.com>
>> Cc: Imre Deak <imre.deak at intel.com>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Reviewed-by: Shashank Sharma <shashank.sharma at intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna at intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_dsb.c | 68 ++++++++++++++++++++++++
>> drivers/gpu/drm/i915/display/intel_dsb.h | 9 ++++
>> 2 files changed, 77 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
>> index f94cd6dc98b6..faa853b08458 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);
>> @@ -102,6 +169,7 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
>> return;
>> }
>>
>> + dsb->ins_start_offset = dsb->free_pos;
> Okay, I'm being a pedant, but that's kind of part of the job
> description, I'm afraid.
>
> What if:
>
> intel_dsb_get()
> intel_dsb_reg_write(dsb, FOO, 0);
> intel_dsb_indexed_reg_write(dsb, FOO, 0);
> intel_dsb_commit()
> intel_dsb_put()
Hi Jani,
I am trying to think a scenario where may write the same register which
is having auto-increment capability using both intel_dsb_reg_write and
intel_dsb_indexed_reg_write.
To set the auto increment mode we may need to write a different register
to control auto-increment mode.
If there is any practical scenario, do you want to me to add now?
Currently checking register value only while creating buffer for
auto-increment register. If we want to add the above then we might need
like below because we are introducing a variability factor regarding
dsb-api to write the same register in consequent call.
enum dsb_write_type {
NORMAL,
INDEX
};
struct last_write {
u32 reg_val;
enum dsb_write_type type;
}
then, last write can be updated in intel_dsb_reg_write() as NORMAL write
and later will check in intel_dsb_indexed_reg_write() to identify above
mentioned case.
Please let me know your suggestion, will do accordingly.
Regards,
Animesh
>
> BR,
> Jani.
>
>> buf[dsb->free_pos++] = val;
>> buf[dsb->free_pos++] = (DSB_OPCODE_MMIO_WRITE << DSB_OPCODE_SHIFT) |
>> (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
>> index 0686d67b34d5..2ae22f7309a7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
>> @@ -30,11 +30,20 @@ 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 and help in identifying the batch 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
More information about the Intel-gfx
mailing list