Re: [Intel-gfx] [PATCH 4/4] drm/i915/dsi: Ungate clock before enabling the phy

Kulkarni, Vandita
2021-10-19T11:32:38Z

> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Tuesday, October 19, 2021 3:48 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Deak, Imre <imre.deak@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>; Kulkarni, Vandita
> <vandita.kulkarni@intel.com>
> Subject: Re: [PATCH 4/4] drm/i915/dsi: Ungate clock before enabling the phy
>
> On Mon, 18 Oct 2021, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> > For the PHY enable/disable signalling to propagate between Dispaly and
> > PHY, DDI clocks need to be running when enabling the PHY.
> >
>
> A bspec reference would be useful:
>
> Bspec: NNN
>
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/icl_dsi.c | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 8c166f92f8bd..77cd01ecfa80 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -1135,8 +1135,6 @@ static void
> > gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
> > const struct intel_crtc_state *crtc_state)
> > {
> > - struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -
> > /* step 4a: power up all lanes of the DDI used by DSI */
> > gen11_dsi_power_up_lanes(encoder);
> >
> > @@ -1146,6 +1144,8 @@ gen11_dsi_enable_port_and_phy(struct
> intel_encoder *encoder,
> > /* step 4c: configure voltage swing and skew */
> > gen11_dsi_voltage_swing_program_seq(encoder);
> >
> > + gen11_dsi_ungate_clocks(encoder);
> > +
> > /* enable DDI buffer */
> > gen11_dsi_enable_ddi_buffer(encoder);
> >
> > @@ -1161,9 +1161,7 @@ gen11_dsi_enable_port_and_phy(struct
> intel_encoder *encoder,
> > /* Step (4h, 4i, 4j, 4k): Configure transcoder */
> > gen11_dsi_configure_transcoder(encoder, crtc_state);
> >
> > - /* Step 4l: Gate DDI clocks */
> > - if (DISPLAY_VER(dev_priv) == 11)
> > - gen11_dsi_gate_clocks(encoder);
> > + gen11_dsi_gate_clocks(encoder);
>
> So how does this relate to
> 991d9557b0c4 ("drm/i915/tgl/dsi: Gate the ddi clocks after pll mapping")

As per the latest bspec, this change doesn't seem to be valid anymore.
It is marked with removed tag.
When TGL got added this change came in.

But now with ADL the whole thing is marked as removed.
So, Do you suggest that I submit a revert for this change ?

Thanks,
Vandita
>
> > }
> >
> > static void gen11_dsi_powerup_panel(struct intel_encoder *encoder)
>
> --
> Jani Nikula, Intel Open Source Graphics Center