[Intel-gfx] [PATCH v5 01/13] drm/i915/icl: Configure lane sequencing of combo phy transmitter

[Chauhan, Madhav]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Thursday, July 19, 2018 9:42 PM
> To: Chauhan, Madhav <madhav.chauhan at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Nikula, Jani <jani.nikula at intel.com>;
> Zanoni, Paulo R <paulo.r.zanoni at intel.com>; Vivi, Rodrigo
> <rodrigo.vivi at intel.com>
> Subject: Re: [Intel-gfx] [PATCH v5 01/13] drm/i915/icl: Configure lane
> sequencing of combo phy transmitter
> 
> On Tue, Jul 10, 2018 at 03:10:02PM +0530, Madhav Chauhan wrote:
> > This patch set the loadgen select and latency optimization for aux and
> > transmit lanes of combo phy transmitters. It will be used for MIPI DSI
> > HS operations.

Thanks for reviewing DSI patches.

> >
> > v2: Rebase
> >
> > Signed-off-by: Madhav Chauhan <madhav.chauhan at intel.com>
> > ---
> >  drivers/gpu/drm/i915/icl_dsi.c | 38
> > ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/icl_dsi.c
> > b/drivers/gpu/drm/i915/icl_dsi.c index 13830e4..a571339 100644
> > --- a/drivers/gpu/drm/i915/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/icl_dsi.c
> > @@ -105,10 +105,48 @@ static void gen11_dsi_power_up_lanes(struct
> intel_encoder *encoder)
> >  	}
> >  }
> >
> > +static void gen11_dsi_config_phy_lanes_sequence(struct intel_encoder
> > +*encoder) {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	enum port port;
> > +	u32 tmp;
> > +	int lane;
> 
> tmp/lane could be moved to into the loops.
> 
> Same in other patches.

Agree, make sense.

> 
> > +
> > +	/* Step 4b(i) set loadgen select for transmit and aux lanes */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		tmp = I915_READ(ICL_PORT_TX_DW4_AUX(port));
> > +		tmp &= ~LOADGEN_SELECT;
> > +		I915_WRITE(ICL_PORT_TX_DW4_AUX(port), tmp);
> > +		for (lane = 0; lane <= 3; lane++) {
> > +			tmp = I915_READ(ICL_PORT_TX_DW4_LN(port,
> lane));
> > +			tmp &= ~LOADGEN_SELECT;
> > +			if (lane != 2)
> > +				tmp |= LOADGEN_SELECT;
> > +			I915_WRITE(ICL_PORT_TX_DW4_LN(port, lane),
> tmp);
> > +		}
> > +	}
> > +
> > +	/* Step 4b(ii) set latency optimization for transmit and aux lanes */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		tmp = I915_READ(ICL_PORT_TX_DW2_AUX(port));
> > +		tmp &= ~FRC_LATENCY_OPTIM_MASK;
> > +		tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
> > +		I915_WRITE(ICL_PORT_TX_DW2_AUX(port), tmp);
> > +		tmp = I915_READ(ICL_PORT_TX_DW2_LN0(port));
> > +		tmp &= ~FRC_LATENCY_OPTIM_MASK;
> > +		tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
> > +		I915_WRITE(ICL_PORT_TX_DW2_GRP(port), tmp);
> > +	}
> 
> An empty line here and there would make this a bit more legible.
> 
> Same in other patches.

Ok.  Thought this will be additional line, multiple
Places in code use this :)

Regards,
Madhav

> 
> > +}
> > +
> >  static void gen11_dsi_enable_port_and_phy(struct intel_encoder
> > *encoder)  {
> >  	/* step 4a: power up all lanes of the DDI used by DSI */
> >  	gen11_dsi_power_up_lanes(encoder);
> > +
> > +	/* step 4b: configure lane sequencing of the Combo-PHY transmitters
> */
> > +	gen11_dsi_config_phy_lanes_sequence(encoder);
> >  }
> >
> >  static void __attribute__((unused))
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel