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

[Chauhan, Madhav]

> -----Original Message-----
> From: Chauhan, Madhav
> Sent: Friday, July 20, 2018 12:06 AM
> To: Ville Syrjälä <ville.syrjala at linux.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
> 
> > -----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.

Was it due to intel_dsi->ports have no port assigned and
loop for_each_dsi_port() will not proceed further??
If that's the case, these encoder enable/disable function should be called
Only when dsi_init is success and then, intel_dsi->ports have some valid port value.

Please clarify.

Regards,
Madhav

> >
> > Same in other patches.
> 
> Agree, make sense.

Just to understand 
> 
> >
> > > +
> > > +	/* 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