diff mbox series

[v7,6/7] drm/i915/dp: Register definition for DP compliance register

Message ID 20200324051111.29398-1-animesh.manna@intel.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Animesh Manna March 24, 2020, 5:11 a.m. UTC 
DP_COMP_CTL and DP_COMP_PAT register used to program DP
compliance pattern.

v1: Initial patch.
v2: used pipe instead of port in macro definition. [Manasi]
v3: used trans_offset for offset calculation. [Manasi]
v4: Used MMIO_PIPE for evenly spaced register offset instead
MMIO_PIPE2. [Ville]

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Navare, Manasi March 27, 2020, 6:45 p.m. UTC | #1 
On Tue, Mar 24, 2020 at 10:41:11AM +0530, Animesh Manna wrote:
> DP_COMP_CTL and DP_COMP_PAT register used to program DP
> compliance pattern.
> 
> v1: Initial patch.
> v2: used pipe instead of port in macro definition. [Manasi]
> v3: used trans_offset for offset calculation. [Manasi]
> v4: Used MMIO_PIPE for evenly spaced register offset instead
> MMIO_PIPE2. [Ville]
> 
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 309cb7d96b35..465862ed2cf8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9792,6 +9792,24 @@ enum skl_power_gate {
>  #define  DDI_BUF_BALANCE_LEG_ENABLE	(1 << 31)
>  #define DDI_BUF_TRANS_HI(port, i)	_MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8 + 4)
>  
> +/* DDI DP Compliance Control */
> +#define _DDI_DP_COMP_CTL_A			0x605F0
> +#define _DDI_DP_COMP_CTL_B			0x615F0
> +#define DDI_DP_COMP_CTL(pipe)			_MMIO_PIPE(pipe, _DDI_DP_COMP_CTL_A, _DDI_DP_COMP_CTL_B)

This looks good now.

> +#define   DDI_DP_COMP_CTL_ENABLE		(1 << 31)
> +#define   DDI_DP_COMP_CTL_D10_2			(0 << 28)
> +#define   DDI_DP_COMP_CTL_SCRAMBLED_0		(1 << 28)
> +#define   DDI_DP_COMP_CTL_PRBS7			(2 << 28)
> +#define   DDI_DP_COMP_CTL_CUSTOM80		(3 << 28)
> +#define   DDI_DP_COMP_CTL_HBR2			(4 << 28)
> +#define   DDI_DP_COMP_CTL_SCRAMBLED_1		(5 << 28)
> +#define   DDI_DP_COMP_CTL_HBR2_RESET		(0xFC << 0)
> +
> +/* DDI DP Compliance Pattern */
> +#define _DDI_DP_COMP_PAT_A			0x605F4
> +#define _DDI_DP_COMP_PAT_B			0x615F4
> +#define DDI_DP_COMP_PAT(pipe, i)		_MMIO(_PIPE(pipe, _DDI_DP_COMP_PAT_A, _DDI_DP_COMP_PAT_B) + (i) * 4)

I still dont understand why we need to use that i argument here and why cant just pipe give us the desired offset
with _MMIO_PIPE(pipe, _DDI_DP_COMP_PAT_A, _DDI_DP_COMP_PAT_B) ?

IMO we should be able to use the above since even here the registers are evenly offseted (0x605F4, 0x615F4, 0x62F54, 0x63F54)

Regards
Manasi

> +
>  /* Sideband Interface (SBI) is programmed indirectly, via
>   * SBI_ADDR, which contains the register offset; and SBI_DATA,
>   * which contains the payload */
> -- 
> 2.24.0
>
Animesh Manna March 30, 2020, 4:01 a.m. UTC | #2 
On 28-03-2020 00:15, Manasi Navare wrote:
> On Tue, Mar 24, 2020 at 10:41:11AM +0530, Animesh Manna wrote:
>> DP_COMP_CTL and DP_COMP_PAT register used to program DP
>> compliance pattern.
>>
>> v1: Initial patch.
>> v2: used pipe instead of port in macro definition. [Manasi]
>> v3: used trans_offset for offset calculation. [Manasi]
>> v4: Used MMIO_PIPE for evenly spaced register offset instead
>> MMIO_PIPE2. [Ville]
>>
>> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 309cb7d96b35..465862ed2cf8 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -9792,6 +9792,24 @@ enum skl_power_gate {
>>   #define  DDI_BUF_BALANCE_LEG_ENABLE	(1 << 31)
>>   #define DDI_BUF_TRANS_HI(port, i)	_MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8 + 4)
>>   
>> +/* DDI DP Compliance Control */
>> +#define _DDI_DP_COMP_CTL_A			0x605F0
>> +#define _DDI_DP_COMP_CTL_B			0x615F0
>> +#define DDI_DP_COMP_CTL(pipe)			_MMIO_PIPE(pipe, _DDI_DP_COMP_CTL_A, _DDI_DP_COMP_CTL_B)
> This looks good now.
>
>> +#define   DDI_DP_COMP_CTL_ENABLE		(1 << 31)
>> +#define   DDI_DP_COMP_CTL_D10_2			(0 << 28)
>> +#define   DDI_DP_COMP_CTL_SCRAMBLED_0		(1 << 28)
>> +#define   DDI_DP_COMP_CTL_PRBS7			(2 << 28)
>> +#define   DDI_DP_COMP_CTL_CUSTOM80		(3 << 28)
>> +#define   DDI_DP_COMP_CTL_HBR2			(4 << 28)
>> +#define   DDI_DP_COMP_CTL_SCRAMBLED_1		(5 << 28)
>> +#define   DDI_DP_COMP_CTL_HBR2_RESET		(0xFC << 0)
>> +
>> +/* DDI DP Compliance Pattern */
>> +#define _DDI_DP_COMP_PAT_A			0x605F4
>> +#define _DDI_DP_COMP_PAT_B			0x615F4
>> +#define DDI_DP_COMP_PAT(pipe, i)		_MMIO(_PIPE(pipe, _DDI_DP_COMP_PAT_A, _DDI_DP_COMP_PAT_B) + (i) * 4)
> I still dont understand why we need to use that i argument here and why cant just pipe give us the desired offset
> with _MMIO_PIPE(pipe, _DDI_DP_COMP_PAT_A, _DDI_DP_COMP_PAT_B) ?
>
> IMO we should be able to use the above since even here the registers are evenly offseted (0x605F4, 0x615F4, 0x62F54, 0x63F54)

The offset you mentioned above is for respective pipe A,B,C,D. How we can write 80 bit custom pattern in it? For pipe A, need to write 0x605F4, 0x605F8, 0x605FC for writing 80 bit .. rt?

Regards,
Animesh

>
> Regards
> Manasi
>
>> +
>>   /* Sideband Interface (SBI) is programmed indirectly, via
>>    * SBI_ADDR, which contains the register offset; and SBI_DATA,
>>    * which contains the payload */
>> -- 
>> 2.24.0
>>
Navare, Manasi March 31, 2020, 12:22 a.m. UTC | #3 
On Mon, Mar 30, 2020 at 09:31:44AM +0530, Manna, Animesh wrote:
> 
> On 28-03-2020 00:15, Manasi Navare wrote:
> >On Tue, Mar 24, 2020 at 10:41:11AM +0530, Animesh Manna wrote:
> >>DP_COMP_CTL and DP_COMP_PAT register used to program DP
> >>compliance pattern.
> >>
> >>v1: Initial patch.
> >>v2: used pipe instead of port in macro definition. [Manasi]
> >>v3: used trans_offset for offset calculation. [Manasi]
> >>v4: Used MMIO_PIPE for evenly spaced register offset instead
> >>MMIO_PIPE2. [Ville]
> >>
> >>Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> >>Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++++
> >>  1 file changed, 18 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>index 309cb7d96b35..465862ed2cf8 100644
> >>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>@@ -9792,6 +9792,24 @@ enum skl_power_gate {
> >>  #define  DDI_BUF_BALANCE_LEG_ENABLE	(1 << 31)
> >>  #define DDI_BUF_TRANS_HI(port, i)	_MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8 + 4)
> >>+/* DDI DP Compliance Control */
> >>+#define _DDI_DP_COMP_CTL_A			0x605F0
> >>+#define _DDI_DP_COMP_CTL_B			0x615F0
> >>+#define DDI_DP_COMP_CTL(pipe)			_MMIO_PIPE(pipe, _DDI_DP_COMP_CTL_A, _DDI_DP_COMP_CTL_B)
> >This looks good now.
> >
> >>+#define   DDI_DP_COMP_CTL_ENABLE		(1 << 31)
> >>+#define   DDI_DP_COMP_CTL_D10_2			(0 << 28)
> >>+#define   DDI_DP_COMP_CTL_SCRAMBLED_0		(1 << 28)
> >>+#define   DDI_DP_COMP_CTL_PRBS7			(2 << 28)
> >>+#define   DDI_DP_COMP_CTL_CUSTOM80		(3 << 28)
> >>+#define   DDI_DP_COMP_CTL_HBR2			(4 << 28)
> >>+#define   DDI_DP_COMP_CTL_SCRAMBLED_1		(5 << 28)
> >>+#define   DDI_DP_COMP_CTL_HBR2_RESET		(0xFC << 0)
> >>+
> >>+/* DDI DP Compliance Pattern */
> >>+#define _DDI_DP_COMP_PAT_A			0x605F4
> >>+#define _DDI_DP_COMP_PAT_B			0x615F4
> >>+#define DDI_DP_COMP_PAT(pipe, i)		_MMIO(_PIPE(pipe, _DDI_DP_COMP_PAT_A, _DDI_DP_COMP_PAT_B) + (i) * 4)
> >I still dont understand why we need to use that i argument here and why cant just pipe give us the desired offset
> >with _MMIO_PIPE(pipe, _DDI_DP_COMP_PAT_A, _DDI_DP_COMP_PAT_B) ?
> >
> >IMO we should be able to use the above since even here the registers are evenly offseted (0x605F4, 0x615F4, 0x62F54, 0x63F54)
> 
> The offset you mentioned above is for respective pipe A,B,C,D. How we can write 80 bit custom pattern in it? For pipe A, need to write 0x605F4, 0x605F8, 0x605FC for writing 80 bit .. rt?
>

Ah okay its for making sure we write 32 bits at a time, got it thanks.

With that

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi
 
> Regards,
> Animesh
> 
> >
> >Regards
> >Manasi
> >
> >>+
> >>  /* Sideband Interface (SBI) is programmed indirectly, via
> >>   * SBI_ADDR, which contains the register offset; and SBI_DATA,
> >>   * which contains the payload */
> >>-- 
> >>2.24.0
> >>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 309cb7d96b35..465862ed2cf8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9792,6 +9792,24 @@  enum skl_power_gate {
 #define  DDI_BUF_BALANCE_LEG_ENABLE	(1 << 31)
 #define DDI_BUF_TRANS_HI(port, i)	_MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8 + 4)
 
+/* DDI DP Compliance Control */
+#define _DDI_DP_COMP_CTL_A			0x605F0
+#define _DDI_DP_COMP_CTL_B			0x615F0
+#define DDI_DP_COMP_CTL(pipe)			_MMIO_PIPE(pipe, _DDI_DP_COMP_CTL_A, _DDI_DP_COMP_CTL_B)
+#define   DDI_DP_COMP_CTL_ENABLE		(1 << 31)
+#define   DDI_DP_COMP_CTL_D10_2			(0 << 28)
+#define   DDI_DP_COMP_CTL_SCRAMBLED_0		(1 << 28)
+#define   DDI_DP_COMP_CTL_PRBS7			(2 << 28)
+#define   DDI_DP_COMP_CTL_CUSTOM80		(3 << 28)
+#define   DDI_DP_COMP_CTL_HBR2			(4 << 28)
+#define   DDI_DP_COMP_CTL_SCRAMBLED_1		(5 << 28)
+#define   DDI_DP_COMP_CTL_HBR2_RESET		(0xFC << 0)
+
+/* DDI DP Compliance Pattern */
+#define _DDI_DP_COMP_PAT_A			0x605F4
+#define _DDI_DP_COMP_PAT_B			0x615F4
+#define DDI_DP_COMP_PAT(pipe, i)		_MMIO(_PIPE(pipe, _DDI_DP_COMP_PAT_A, _DDI_DP_COMP_PAT_B) + (i) * 4)
+
 /* Sideband Interface (SBI) is programmed indirectly, via
  * SBI_ADDR, which contains the register offset; and SBI_DATA,
  * which contains the payload */