[v2,05/15] drm/i915/dsb: Indexed register write function for DSB.
diff mbox series

Message ID 20190821063236.19705-6-animesh.manna@intel.com
State New
Headers show
Series
  • DSB enablement.
Related show

Commit Message

Animesh Manna Aug. 21, 2019, 6:32 a.m. UTC
DSB can program large set of data through indexed register write
(opcode 0x9) in one shot. Will be using for bulk register programming
e.g. gamma lut programming, HDR meta data programming.

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>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 42 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h |  6 ++++
 2 files changed, 48 insertions(+)

Comments

Chris Wilson Aug. 21, 2019, 6:27 p.m. UTC | #1
Quoting Animesh Manna (2019-08-21 07:32:25)
> DSB can program large set of data through indexed register write
> (opcode 0x9) in one shot. Will be using for bulk register programming
> e.g. gamma lut programming, HDR meta data programming.
> 
> 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>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 42 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dsb.h |  6 ++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 8a9d082b1601..4fe8cac6246a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -22,6 +22,7 @@
>  #define DSB_OPCODE_INDEXED_WRITE       0x9
>  #define DSB_OPCODE_POLL                        0xA
>  #define DSB_BYTE_EN                    (0xf << 20)
> +#define DSB_REG_VALUE_MASK             0xfffff
>  
>  struct intel_dsb *
>  intel_dsb_get(struct intel_crtc *crtc)
> @@ -79,6 +80,42 @@ intel_dsb_get(struct intel_crtc *crtc)
>         return dsb;
>  }
>  
> +static void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
> +                                       u32 val)
> +{
> +       u32 *buf = dsb->cmd_buf;
> +       u32 reg_val;
> +
> +       reg_val = buf[dsb->ins_start_offset + 1] & DSB_REG_VALUE_MASK;

Uncached read.

> +       if (reg_val != i915_mmio_reg_offset(reg)) {
> +               /* Every instruction should be 8 byte aligned. */
> +               if (dsb->free_pos & 0x1)
> +                       dsb->free_pos++;

dsb->free_pos = ALIGN(dsb->free_pos, 2);

> +
> +               /* Update the size. */
> +               dsb->ins_start_offset = dsb->free_pos;
> +               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]++;

Uncached read and write. So far this is working out to be _more_
expensive than mmio.
-Chris
Animesh Manna Aug. 22, 2019, 12:06 p.m. UTC | #2
Hi,


On 8/21/2019 11:57 PM, Chris Wilson wrote:
> Quoting Animesh Manna (2019-08-21 07:32:25)
>> DSB can program large set of data through indexed register write
>> (opcode 0x9) in one shot. Will be using for bulk register programming
>> e.g. gamma lut programming, HDR meta data programming.
>>
>> 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>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dsb.c | 42 ++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_dsb.h |  6 ++++
>>   2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
>> index 8a9d082b1601..4fe8cac6246a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -22,6 +22,7 @@
>>   #define DSB_OPCODE_INDEXED_WRITE       0x9
>>   #define DSB_OPCODE_POLL                        0xA
>>   #define DSB_BYTE_EN                    (0xf << 20)
>> +#define DSB_REG_VALUE_MASK             0xfffff
>>   
>>   struct intel_dsb *
>>   intel_dsb_get(struct intel_crtc *crtc)
>> @@ -79,6 +80,42 @@ intel_dsb_get(struct intel_crtc *crtc)
>>          return dsb;
>>   }
>>   
>> +static void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
>> +                                       u32 val)
>> +{
>> +       u32 *buf = dsb->cmd_buf;
>> +       u32 reg_val;
>> +
>> +       reg_val = buf[dsb->ins_start_offset + 1] & DSB_REG_VALUE_MASK;
> Uncached read.
>
>> +       if (reg_val != i915_mmio_reg_offset(reg)) {
>> +               /* Every instruction should be 8 byte aligned. */
>> +               if (dsb->free_pos & 0x1)
>> +                       dsb->free_pos++;
> dsb->free_pos = ALIGN(dsb->free_pos, 2);

Ok.
>
>> +
>> +               /* Update the size. */
>> +               dsb->ins_start_offset = dsb->free_pos;
>> +               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]++;
> Uncached read and write. So far this is working out to be _more_
> expensive than mmio.

Understood your concern, currently tried to align with I915_WRITE() call 
and coded accordingly.
If we can introduce a separate call for auto-increment register like below.

I915_WRITE_BULK(<reg-offset>, <buf-start-address>, <count>);

The implementation will be simpler and more optimized.
Good to know your feedback or any better way can be done.

Regards,
Animesh
> -Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 8a9d082b1601..4fe8cac6246a 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -22,6 +22,7 @@ 
 #define DSB_OPCODE_INDEXED_WRITE	0x9
 #define DSB_OPCODE_POLL			0xA
 #define DSB_BYTE_EN			(0xf << 20)
+#define DSB_REG_VALUE_MASK		0xfffff
 
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
@@ -79,6 +80,42 @@  intel_dsb_get(struct intel_crtc *crtc)
 	return dsb;
 }
 
+static void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
+					u32 val)
+{
+	u32 *buf = dsb->cmd_buf;
+	u32 reg_val;
+
+	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. */
+		if (dsb->free_pos & 0x1)
+			dsb->free_pos++;
+
+		/* Update the size. */
+		dsb->ins_start_offset = dsb->free_pos;
+		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 = dsb->crtc;
@@ -95,6 +132,11 @@  void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
 		return;
 	}
 
+	if (reg.cap == DSB_INDEX_WRITE) {
+		intel_dsb_indexed_reg_write(dsb, reg, val);
+		return;
+	}
+
 	buf[dsb->free_pos++] = val;
 	buf[dsb->free_pos++] = (DSB_OPCODE_MMIO_WRITE  <<
 				DSB_OPCODE_SHIFT) | DSB_BYTE_EN |
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 2015c372b0d5..1fa893cc8c2e 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -31,6 +31,12 @@  struct intel_dsb {
 	 * and help in calculating cmd_buf_tail.
 	 */
 	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 *