| Message ID | 1553076539-12556-1-git-send-email-vandita.kulkarni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/icl/dsi: Ungate clocks if gated | expand |
>-----Original Message----- >From: Kulkarni, Vandita >Sent: Wednesday, March 20, 2019 3:39 PM >To: intel-gfx@lists.freedesktop.org >Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma <uma.shankar@intel.com>; >Chauhan, Madhav <madhav.chauhan@intel.com>; Kulkarni, Vandita ><vandita.kulkarni@intel.com> >Subject: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated You can drop dsi from commit header. Just drm/i915/icl/ should be good. Also update header as Ungate ddi clocks if gated > >IO enable sequencing needs ddi clocks enabled. >These clocks will be gated at the later point in the enable sequence. > >Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> >--- > drivers/gpu/drm/i915/icl_dsi.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > >diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c index >beb30d9..f02504d 100644 >--- a/drivers/gpu/drm/i915/icl_dsi.c >+++ b/drivers/gpu/drm/i915/icl_dsi.c >@@ -589,6 +589,14 @@ static void gen11_dsi_map_pll(struct intel_encoder >*encoder, > val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); > } > I915_WRITE(DPCLKA_CFGCR0_ICL, val); >+ >+ /* make sure that the ddi clocks are not gated */ >+ val = I915_READ(DPCLKA_CFGCR0_ICL); >+ for_each_dsi_port(port, intel_dsi->ports) { >+ val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); >+ } >+ I915_WRITE(DPCLKA_CFGCR0_ICL, val); >+ > POSTING_READ(DPCLKA_CFGCR0_ICL); I think you can reuse the val from top and avoid an extra write to the same register. Otherwise change looks ok to me. With above comments fixed, Reviewed-by: Uma Shankar <uma.shankar@intel.com> > > mutex_unlock(&dev_priv->dpll_lock); >-- >1.9.1
> -----Original Message----- > From: Shankar, Uma > Sent: Wednesday, March 20, 2019 5:19 PM > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Nikula, Jani <jani.nikula@intel.com>; Chauhan, Madhav > <madhav.chauhan@intel.com> > Subject: RE: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated > > > > >-----Original Message----- > >From: Kulkarni, Vandita > >Sent: Wednesday, March 20, 2019 3:39 PM > >To: intel-gfx@lists.freedesktop.org > >Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma > ><uma.shankar@intel.com>; Chauhan, Madhav <madhav.chauhan@intel.com>; > >Kulkarni, Vandita <vandita.kulkarni@intel.com> > >Subject: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated > > You can drop dsi from commit header. Just drm/i915/icl/ should be good. > Also update header as Ungate ddi clocks if gated Okay. > > > > >IO enable sequencing needs ddi clocks enabled. > >These clocks will be gated at the later point in the enable sequence. > > > >Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> > >--- > > drivers/gpu/drm/i915/icl_dsi.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > >diff --git a/drivers/gpu/drm/i915/icl_dsi.c > >b/drivers/gpu/drm/i915/icl_dsi.c index beb30d9..f02504d 100644 > >--- a/drivers/gpu/drm/i915/icl_dsi.c > >+++ b/drivers/gpu/drm/i915/icl_dsi.c > >@@ -589,6 +589,14 @@ static void gen11_dsi_map_pll(struct intel_encoder > >*encoder, > > val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); > > } > > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > >+ > >+ /* make sure that the ddi clocks are not gated */ > >+ val = I915_READ(DPCLKA_CFGCR0_ICL); > >+ for_each_dsi_port(port, intel_dsi->ports) { > >+ val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); > >+ } > >+ I915_WRITE(DPCLKA_CFGCR0_ICL, val); > >+ > > POSTING_READ(DPCLKA_CFGCR0_ICL); > > I think you can reuse the val from top and avoid an extra write to the same > register. At this point we ideally have the clocks gated and we need to ungate it. We must write to this register. Accordingly, will fix the commit header too. Thanks. Vandita > > Otherwise change looks ok to me. With above comments fixed, > Reviewed-by: Uma Shankar <uma.shankar@intel.com> > > > > > mutex_unlock(&dev_priv->dpll_lock); > >-- > >1.9.1
> -----Original Message----- > From: Kulkarni, Vandita > Sent: Thursday, March 21, 2019 7:23 PM > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Nikula, Jani <jani.nikula@intel.com>; Chauhan, Madhav > <madhav.chauhan@intel.com> > Subject: RE: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated > > > > -----Original Message----- > > From: Shankar, Uma > > Sent: Wednesday, March 20, 2019 5:19 PM > > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: Nikula, Jani <jani.nikula@intel.com>; Chauhan, Madhav > > <madhav.chauhan@intel.com> > > Subject: RE: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated > > > > > > > > >-----Original Message----- > > >From: Kulkarni, Vandita > > >Sent: Wednesday, March 20, 2019 3:39 PM > > >To: intel-gfx@lists.freedesktop.org > > >Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma > > ><uma.shankar@intel.com>; Chauhan, Madhav > <madhav.chauhan@intel.com>; > > >Kulkarni, Vandita <vandita.kulkarni@intel.com> > > >Subject: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated > > > > You can drop dsi from commit header. Just drm/i915/icl/ should be good. > > Also update header as Ungate ddi clocks if gated > Okay. > > > > > > > >IO enable sequencing needs ddi clocks enabled. > > >These clocks will be gated at the later point in the enable sequence. > > > > > >Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> > > >--- > > > drivers/gpu/drm/i915/icl_dsi.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > >diff --git a/drivers/gpu/drm/i915/icl_dsi.c > > >b/drivers/gpu/drm/i915/icl_dsi.c index beb30d9..f02504d 100644 > > >--- a/drivers/gpu/drm/i915/icl_dsi.c > > >+++ b/drivers/gpu/drm/i915/icl_dsi.c > > >@@ -589,6 +589,14 @@ static void gen11_dsi_map_pll(struct > > >intel_encoder *encoder, > > > val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); > > > } > > > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > >+ > > >+ /* make sure that the ddi clocks are not gated */ > > >+ val = I915_READ(DPCLKA_CFGCR0_ICL); > > >+ for_each_dsi_port(port, intel_dsi->ports) { > > >+ val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); > > >+ } > > >+ I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > >+ > > > POSTING_READ(DPCLKA_CFGCR0_ICL); > > > > I think you can reuse the val from top and avoid an extra write to the > > same register. > At this point we ideally have the clocks gated and we need to ungate it. We must > write to this register. > Accordingly, will fix the commit header too. As per the spec, 2 different writes are needed for mapping and turning on the clocks. Thanks, Vandita > > Thanks. > Vandita > > > > Otherwise change looks ok to me. With above comments fixed, > > Reviewed-by: Uma Shankar <uma.shankar@intel.com> > > > > > > > > mutex_unlock(&dev_priv->dpll_lock); > > >-- > > >1.9.1
>> -----Original Message----- >> From: Kulkarni, Vandita >> Sent: Thursday, March 21, 2019 7:23 PM >> To: Shankar, Uma <uma.shankar@intel.com>; >> intel-gfx@lists.freedesktop.org >> Cc: Nikula, Jani <jani.nikula@intel.com>; Chauhan, Madhav >> <madhav.chauhan@intel.com> >> Subject: RE: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated >> >> >> > -----Original Message----- >> > From: Shankar, Uma >> > Sent: Wednesday, March 20, 2019 5:19 PM >> > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel- >> > gfx@lists.freedesktop.org >> > Cc: Nikula, Jani <jani.nikula@intel.com>; Chauhan, Madhav >> > <madhav.chauhan@intel.com> >> > Subject: RE: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated >> > >> > >> > >> > >-----Original Message----- >> > >From: Kulkarni, Vandita >> > >Sent: Wednesday, March 20, 2019 3:39 PM >> > >To: intel-gfx@lists.freedesktop.org >> > >Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma >> > ><uma.shankar@intel.com>; Chauhan, Madhav >> <madhav.chauhan@intel.com>; >> > >Kulkarni, Vandita <vandita.kulkarni@intel.com> >> > >Subject: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated >> > >> > You can drop dsi from commit header. Just drm/i915/icl/ should be good. >> > Also update header as Ungate ddi clocks if gated >> Okay. >> > >> > > >> > >IO enable sequencing needs ddi clocks enabled. >> > >These clocks will be gated at the later point in the enable sequence. >> > > >> > >Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> >> > >--- >> > > drivers/gpu/drm/i915/icl_dsi.c | 8 ++++++++ >> > > 1 file changed, 8 insertions(+) >> > > >> > >diff --git a/drivers/gpu/drm/i915/icl_dsi.c >> > >b/drivers/gpu/drm/i915/icl_dsi.c index beb30d9..f02504d 100644 >> > >--- a/drivers/gpu/drm/i915/icl_dsi.c >> > >+++ b/drivers/gpu/drm/i915/icl_dsi.c >> > >@@ -589,6 +589,14 @@ static void gen11_dsi_map_pll(struct >> > >intel_encoder *encoder, >> > > val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); >> > > } >> > > I915_WRITE(DPCLKA_CFGCR0_ICL, val); >> > >+ >> > >+ /* make sure that the ddi clocks are not gated */ >> > >+ val = I915_READ(DPCLKA_CFGCR0_ICL); >> > >+ for_each_dsi_port(port, intel_dsi->ports) { >> > >+ val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); >> > >+ } >> > >+ I915_WRITE(DPCLKA_CFGCR0_ICL, val); >> > >+ >> > > POSTING_READ(DPCLKA_CFGCR0_ICL); >> > >> > I think you can reuse the val from top and avoid an extra write to >> > the same register. >> At this point we ideally have the clocks gated and we need to ungate >> it. We must write to this register. >> Accordingly, will fix the commit header too. >As per the spec, 2 different writes are needed for mapping and turning on the clocks. Oh ok, yes seems like indeed spec expects separate write for individual steps. Thanks for the pointing out. You can keep the current change and my RB. Regards, Uma Shankar >Thanks, >Vandita > >> >> Thanks. >> Vandita >> > >> > Otherwise change looks ok to me. With above comments fixed, >> > Reviewed-by: Uma Shankar <uma.shankar@intel.com> >> > >> > > >> > > mutex_unlock(&dev_priv->dpll_lock); >> > >-- >> > >1.9.1
diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c index beb30d9..f02504d 100644 --- a/drivers/gpu/drm/i915/icl_dsi.c +++ b/drivers/gpu/drm/i915/icl_dsi.c @@ -589,6 +589,14 @@ static void gen11_dsi_map_pll(struct intel_encoder *encoder, val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); } I915_WRITE(DPCLKA_CFGCR0_ICL, val); + + /* make sure that the ddi clocks are not gated */ + val = I915_READ(DPCLKA_CFGCR0_ICL); + for_each_dsi_port(port, intel_dsi->ports) { + val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); + } + I915_WRITE(DPCLKA_CFGCR0_ICL, val); + POSTING_READ(DPCLKA_CFGCR0_ICL); mutex_unlock(&dev_priv->dpll_lock);
IO enable sequencing needs ddi clocks enabled. These clocks will be gated at the later point in the enable sequence. Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> --- drivers/gpu/drm/i915/icl_dsi.c | 8 ++++++++ 1 file changed, 8 insertions(+)