| Message ID | 20211018065207.30587-5-vandita.kulkarni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable MIPI DSI video mode on ADLP | expand |
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") > } > > static void gen11_dsi_powerup_panel(struct intel_encoder *encoder)
> -----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
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of > Kulkarni, Vandita > Sent: Tuesday, October 19, 2021 5:03 PM > To: Nikula, Jani <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Deak, Imre <imre.deak@intel.com>; Roper, Matthew D > <matthew.d.roper@intel.com> > Subject: Re: [Intel-gfx] [PATCH 4/4] drm/i915/dsi: Ungate clock before > enabling the phy > > > -----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 Bspec: 49187 > > > > > 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. And the gating is now added after enabling DDI Buffer > 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
On Tue, 19 Oct 2021, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote: >> -----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 ? No, just an explanation and maybe that commit reference in the commit message. BR, Jani. > > Thanks, > Vandita >> >> > } >> > >> > static void gen11_dsi_powerup_panel(struct intel_encoder *encoder) >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
> -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: Tuesday, October 19, 2021 5:16 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> > Subject: RE: [PATCH 4/4] drm/i915/dsi: Ungate clock before enabling the phy > > On Tue, 19 Oct 2021, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> > wrote: > >> -----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 ? > > No, just an explanation and maybe that commit reference in the commit > message. Okay, will do that. Thanks, Vandita > > BR, > Jani. > > > > > Thanks, > > Vandita > >> > >> > } > >> > > >> > static void gen11_dsi_powerup_panel(struct intel_encoder *encoder) > >> > >> -- > >> Jani Nikula, Intel Open Source Graphics Center > > -- > Jani Nikula, Intel Open Source Graphics Center
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); } static void gen11_dsi_powerup_panel(struct intel_encoder *encoder)
For the PHY enable/disable signalling to propagate between Dispaly and PHY, DDI clocks need to be running when enabling the PHY. 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(-)