subject:"\[Intel\-gfx\] \[PATCH v2 06\/21\] drm\/i915\/mtl\: Add vswing programming for C10 phys"

Re: [Intel-gfx] [PATCH v2 06/21] drm/i915/mtl: Add vswing programming for C10 phys

2023-02-07 Thread Gustavo Sousa

On Tue, Feb 07, 2023 at 01:52:20PM -0300, Gustavo Sousa wrote:
> I feel like some parts of this patch actually belong to previous patches as
> fixups instead, namely the implementation and usage of
> intel_cx0_phy_transaction_{begin,end}() and other fixes unrelated to vswing
> programming.
> 
> Also, please see some comments inline.
> 
> On Thu, Jan 05, 2023 at 02:54:31PM +0200, Mika Kahola wrote:
> > From: Radhakrishna Sripada 
> > 
> > C10 phys uses direct mapping internally for voltage and pre-emphasis levels.
> > Program the levels directly to the fields in the VDR Registers.
> > 
> > Bspec: 65449
> > 
> > v2: From table "C10: Tx EQ settings for DP 1.4x" it shows level 1
> > and preemphasis 1 instead of two times of level 1 preemphasis 0.
> > Fix this in the driver code as well.
> > 
> > Cc: Imre Deak 
> > Cc: Uma Shankar 
> > Signed-off-by: Clint Taylor 
> > Signed-off-by: Radhakrishna Sripada 
> > Signed-off-by: Mika Kahola 
> > Link: 
> > https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-7-mika.kah...@intel.com
> > ---
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 131 --
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.h  |   2 +
> >  .../gpu/drm/i915/display/intel_cx0_phy_regs.h |   6 +
> >  drivers/gpu/drm/i915/display/intel_ddi.c  |   4 +-
> >  .../drm/i915/display/intel_ddi_buf_trans.c|  36 -
> >  .../drm/i915/display/intel_ddi_buf_trans.h|   6 +
> >  .../i915/display/intel_display_power_map.c|   1 +
> >  7 files changed, 175 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c 
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index 746c905538a7..3d86b0f1c36d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -6,10 +6,14 @@
> >  #include "i915_reg.h"
> >  #include "intel_cx0_phy.h"
> >  #include "intel_cx0_phy_regs.h"
> > +#include "intel_ddi.h"
> > +#include "intel_ddi_buf_trans.h"
> >  #include "intel_de.h"
> >  #include "intel_display_types.h"
> >  #include "intel_dp.h"
> >  #include "intel_panel.h"
> > +#include "intel_psr.h"
> > +#include "intel_uncore.h"
> >  
> >  bool intel_is_c10phy(struct drm_i915_private *dev_priv, enum phy phy)
> >  {
> > @@ -19,6 +23,15 @@ bool intel_is_c10phy(struct drm_i915_private *dev_priv, 
> > enum phy phy)
> > return false;
> >  }
> >  
> > +static void
> > +assert_dc_off(struct drm_i915_private *i915)
> > +{
> > +   bool enabled;
> > +
> > +   enabled = intel_display_power_is_enabled(i915, POWER_DOMAIN_DC_OFF);
> > +   drm_WARN_ON(>drm, !enabled);
> > +}
> > +
> >  static void intel_cx0_bus_reset(struct drm_i915_private *i915, enum port 
> > port, int lane)
> >  {
> > enum phy phy = intel_port_to_phy(i915, port);
> > @@ -111,6 +124,8 @@ static u8 intel_cx0_read(struct drm_i915_private *i915, 
> > enum port port,
> > int i, status = 0;
> > u32 val;
> >  
> > +   assert_dc_off(i915);
> > +
> > for (i = 0; i < 3; i++) {
> > status = __intel_cx0_read(i915, port, lane, addr, );
> >  
> > @@ -193,6 +208,8 @@ static void __intel_cx0_write(struct drm_i915_private 
> > *i915, enum port port,
> > enum phy phy = intel_port_to_phy(i915, port);
> > int i, status;
> >  
> > +   assert_dc_off(i915);
> > +
> > for (i = 0; i < 3; i++) {
> > status = __intel_cx0_write_once(i915, port, lane, addr, data, 
> > committed);
> >  
> > @@ -240,6 +257,80 @@ static void intel_cx0_rmw(struct drm_i915_private 
> > *i915, enum port port,
> > }
> >  }
> >  
> > +/*
> > + * Prepare HW for CX0 phy transactions.
> > + *
> > + * It is required that PSR and DC5/6 are disabled before any CX0 message
> > + * bus transaction is executed.
> > + */
> > +static intel_wakeref_t intel_cx0_phy_transaction_begin(struct 
> > intel_encoder *encoder)
> > +{
> > +   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > +   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +
> > +   intel_psr_pause(intel_dp);
> 
> Shouldn't we check if this encoder is really for DP before calling
> intel_psr_{pause,resume}()?
> 
> > +   return intel_display_power_get(i915, POWER_DOMAIN_DC_OFF);
> > +}
> > +
> > +static void intel_cx0_phy_transaction_end(struct intel_encoder *encoder, 
> > intel_wakeref_t wakeref)
> > +{
> > +   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > +   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +
> > +   intel_psr_resume(intel_dp);
> > +   intel_display_power_put(i915, POWER_DOMAIN_DC_OFF, wakeref);
> > +}
> > +
> > +void intel_cx0_phy_set_signal_levels(struct intel_encoder *encoder,
> > +const struct intel_crtc_state *crtc_state)
> > +{
> > +   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > +   struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > +   bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> > +   u8

Re: [Intel-gfx] [PATCH v2 06/21] drm/i915/mtl: Add vswing programming for C10 phys

2023-02-07 Thread Gustavo Sousa

I feel like some parts of this patch actually belong to previous patches as
fixups instead, namely the implementation and usage of
intel_cx0_phy_transaction_{begin,end}() and other fixes unrelated to vswing
programming.

Also, please see some comments inline.

On Thu, Jan 05, 2023 at 02:54:31PM +0200, Mika Kahola wrote:
> From: Radhakrishna Sripada 
> 
> C10 phys uses direct mapping internally for voltage and pre-emphasis levels.
> Program the levels directly to the fields in the VDR Registers.
> 
> Bspec: 65449
> 
> v2: From table "C10: Tx EQ settings for DP 1.4x" it shows level 1
> and preemphasis 1 instead of two times of level 1 preemphasis 0.
> Fix this in the driver code as well.
> 
> Cc: Imre Deak 
> Cc: Uma Shankar 
> Signed-off-by: Clint Taylor 
> Signed-off-by: Radhakrishna Sripada 
> Signed-off-by: Mika Kahola 
> Link: 
> https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-7-mika.kah...@intel.com
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 131 --
>  drivers/gpu/drm/i915/display/intel_cx0_phy.h  |   2 +
>  .../gpu/drm/i915/display/intel_cx0_phy_regs.h |   6 +
>  drivers/gpu/drm/i915/display/intel_ddi.c  |   4 +-
>  .../drm/i915/display/intel_ddi_buf_trans.c|  36 -
>  .../drm/i915/display/intel_ddi_buf_trans.h|   6 +
>  .../i915/display/intel_display_power_map.c|   1 +
>  7 files changed, 175 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c 
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 746c905538a7..3d86b0f1c36d 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -6,10 +6,14 @@
>  #include "i915_reg.h"
>  #include "intel_cx0_phy.h"
>  #include "intel_cx0_phy_regs.h"
> +#include "intel_ddi.h"
> +#include "intel_ddi_buf_trans.h"
>  #include "intel_de.h"
>  #include "intel_display_types.h"
>  #include "intel_dp.h"
>  #include "intel_panel.h"
> +#include "intel_psr.h"
> +#include "intel_uncore.h"
>  
>  bool intel_is_c10phy(struct drm_i915_private *dev_priv, enum phy phy)
>  {
> @@ -19,6 +23,15 @@ bool intel_is_c10phy(struct drm_i915_private *dev_priv, 
> enum phy phy)
>   return false;
>  }
>  
> +static void
> +assert_dc_off(struct drm_i915_private *i915)
> +{
> + bool enabled;
> +
> + enabled = intel_display_power_is_enabled(i915, POWER_DOMAIN_DC_OFF);
> + drm_WARN_ON(>drm, !enabled);
> +}
> +
>  static void intel_cx0_bus_reset(struct drm_i915_private *i915, enum port 
> port, int lane)
>  {
>   enum phy phy = intel_port_to_phy(i915, port);
> @@ -111,6 +124,8 @@ static u8 intel_cx0_read(struct drm_i915_private *i915, 
> enum port port,
>   int i, status = 0;
>   u32 val;
>  
> + assert_dc_off(i915);
> +
>   for (i = 0; i < 3; i++) {
>   status = __intel_cx0_read(i915, port, lane, addr, );
>  
> @@ -193,6 +208,8 @@ static void __intel_cx0_write(struct drm_i915_private 
> *i915, enum port port,
>   enum phy phy = intel_port_to_phy(i915, port);
>   int i, status;
>  
> + assert_dc_off(i915);
> +
>   for (i = 0; i < 3; i++) {
>   status = __intel_cx0_write_once(i915, port, lane, addr, data, 
> committed);
>  
> @@ -240,6 +257,80 @@ static void intel_cx0_rmw(struct drm_i915_private *i915, 
> enum port port,
>   }
>  }
>  
> +/*
> + * Prepare HW for CX0 phy transactions.
> + *
> + * It is required that PSR and DC5/6 are disabled before any CX0 message
> + * bus transaction is executed.
> + */
> +static intel_wakeref_t intel_cx0_phy_transaction_begin(struct intel_encoder 
> *encoder)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +
> + intel_psr_pause(intel_dp);

Shouldn't we check if this encoder is really for DP before calling
intel_psr_{pause,resume}()?

> + return intel_display_power_get(i915, POWER_DOMAIN_DC_OFF);
> +}
> +
> +static void intel_cx0_phy_transaction_end(struct intel_encoder *encoder, 
> intel_wakeref_t wakeref)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +
> + intel_psr_resume(intel_dp);
> + intel_display_power_put(i915, POWER_DOMAIN_DC_OFF, wakeref);
> +}
> +
> +void intel_cx0_phy_set_signal_levels(struct intel_encoder *encoder,
> +  const struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> + bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> + u8 master_lane = lane_reversal ? INTEL_CX0_LANE1 :
> +  INTEL_CX0_LANE0;
> + u8 follower_lane = lane_reversal ? INTEL_CX0_LANE0 :
> +  INTEL_CX0_LANE1;
> + const struct intel_ddi_buf_trans *trans;
> +

[Intel-gfx] [PATCH v2 06/21] drm/i915/mtl: Add vswing programming for C10 phys

2023-01-05 Thread Mika Kahola

From: Radhakrishna Sripada 

C10 phys uses direct mapping internally for voltage and pre-emphasis levels.
Program the levels directly to the fields in the VDR Registers.

Bspec: 65449

v2: From table "C10: Tx EQ settings for DP 1.4x" it shows level 1
and preemphasis 1 instead of two times of level 1 preemphasis 0.
Fix this in the driver code as well.

Cc: Imre Deak 
Cc: Uma Shankar 
Signed-off-by: Clint Taylor 
Signed-off-by: Radhakrishna Sripada 
Signed-off-by: Mika Kahola 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-7-mika.kah...@intel.com
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 131 --
 drivers/gpu/drm/i915/display/intel_cx0_phy.h  |   2 +
 .../gpu/drm/i915/display/intel_cx0_phy_regs.h |   6 +
 drivers/gpu/drm/i915/display/intel_ddi.c  |   4 +-
 .../drm/i915/display/intel_ddi_buf_trans.c|  36 -
 .../drm/i915/display/intel_ddi_buf_trans.h|   6 +
 .../i915/display/intel_display_power_map.c|   1 +
 7 files changed, 175 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c 
b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 746c905538a7..3d86b0f1c36d 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -6,10 +6,14 @@
 #include "i915_reg.h"
 #include "intel_cx0_phy.h"
 #include "intel_cx0_phy_regs.h"
+#include "intel_ddi.h"
+#include "intel_ddi_buf_trans.h"
 #include "intel_de.h"
 #include "intel_display_types.h"
 #include "intel_dp.h"
 #include "intel_panel.h"
+#include "intel_psr.h"
+#include "intel_uncore.h"
 
 bool intel_is_c10phy(struct drm_i915_private *dev_priv, enum phy phy)
 {
@@ -19,6 +23,15 @@ bool intel_is_c10phy(struct drm_i915_private *dev_priv, enum 
phy phy)
return false;
 }
 
+static void
+assert_dc_off(struct drm_i915_private *i915)
+{
+   bool enabled;
+
+   enabled = intel_display_power_is_enabled(i915, POWER_DOMAIN_DC_OFF);
+   drm_WARN_ON(>drm, !enabled);
+}
+
 static void intel_cx0_bus_reset(struct drm_i915_private *i915, enum port port, 
int lane)
 {
enum phy phy = intel_port_to_phy(i915, port);
@@ -111,6 +124,8 @@ static u8 intel_cx0_read(struct drm_i915_private *i915, 
enum port port,
int i, status = 0;
u32 val;
 
+   assert_dc_off(i915);
+
for (i = 0; i < 3; i++) {
status = __intel_cx0_read(i915, port, lane, addr, );
 
@@ -193,6 +208,8 @@ static void __intel_cx0_write(struct drm_i915_private 
*i915, enum port port,
enum phy phy = intel_port_to_phy(i915, port);
int i, status;
 
+   assert_dc_off(i915);
+
for (i = 0; i < 3; i++) {
status = __intel_cx0_write_once(i915, port, lane, addr, data, 
committed);
 
@@ -240,6 +257,80 @@ static void intel_cx0_rmw(struct drm_i915_private *i915, 
enum port port,
}
 }
 
+/*
+ * Prepare HW for CX0 phy transactions.
+ *
+ * It is required that PSR and DC5/6 are disabled before any CX0 message
+ * bus transaction is executed.
+ */
+static intel_wakeref_t intel_cx0_phy_transaction_begin(struct intel_encoder 
*encoder)
+{
+   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+   intel_psr_pause(intel_dp);
+   return intel_display_power_get(i915, POWER_DOMAIN_DC_OFF);
+}
+
+static void intel_cx0_phy_transaction_end(struct intel_encoder *encoder, 
intel_wakeref_t wakeref)
+{
+   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+   intel_psr_resume(intel_dp);
+   intel_display_power_put(i915, POWER_DOMAIN_DC_OFF, wakeref);
+}
+
+void intel_cx0_phy_set_signal_levels(struct intel_encoder *encoder,
+const struct intel_crtc_state *crtc_state)
+{
+   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+   struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
+   bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
+   u8 master_lane = lane_reversal ? INTEL_CX0_LANE1 :
+INTEL_CX0_LANE0;
+   u8 follower_lane = lane_reversal ? INTEL_CX0_LANE0 :
+INTEL_CX0_LANE1;
+   const struct intel_ddi_buf_trans *trans;
+   intel_wakeref_t wakeref;
+   int n_entries, ln;
+
+   wakeref = intel_cx0_phy_transaction_begin(encoder);
+
+   trans = encoder->get_buf_trans(encoder, crtc_state, _entries);
+   if (drm_WARN_ON_ONCE(>drm, !trans))
+   return;
+
+   intel_cx0_rmw(i915, encoder->port, INTEL_CX0_BOTH_LANES, 
PHY_C10_VDR_CONTROL(1),
+ 0, C10_VDR_CTRL_MSGBUS_ACCESS, MB_WRITE_COMMITTED);
+
+   for (ln = 0; ln < 4; ln++) {
+   int level = intel_ddi_level(encoder, crtc_state, ln);
+   int lane, tx;
+
+   lane = ln / 2 + 1;
+

Re: [Intel-gfx] [PATCH v2 06/21] drm/i915/mtl: Add vswing programming for C10 phys

Re: [Intel-gfx] [PATCH v2 06/21] drm/i915/mtl: Add vswing programming for C10 phys

[Intel-gfx] [PATCH v2 06/21] drm/i915/mtl: Add vswing programming for C10 phys

3 matches

Site Navigation

Mail list logo

Footer information