Re: [Intel-gfx] [PATCH v3] drm/i915/glk: Reuse broxton code for geminilake

[Vivi, Rodrigo]

On Fri, 2016-12-02 at 10:27 +0200, Ander Conselvan De Oliveira wrote:
> On Thu, 2016-12-01 at 17:06 -0800, Rodrigo Vivi wrote:
> > A reviewed backwards because I was willing to check if all ifs were in 
> > place.
> > 
> > I missed the ones from i915_drv.c
> > 
> > *** i915_drv.c:
> > i915_drm_suspend_late[1500]fw_csr = !IS_GEN9_LP(dev_priv) &&
> > i915_drm_suspend_late[1513]if (IS_GEN9_LP(dev_priv))
> > i915_drm_resume_early[1721]if (IS_GEN9_LP(dev_priv)) {
> > i915_drm_resume_early[1731]if (IS_GEN9_LP(dev_priv) ||
> > intel_runtime_suspend[2354]if (IS_GEN9_LP(dev_priv)) {
> > intel_runtime_resume[2439] if (IS_GEN9_LP(dev_priv)) {
> > 
> > The ones that I had added with the patch
> > "drm/i915/glk: Add missing bits to allow runtime pm suspend on GLK."
> > 
> > Feel free to squash all those into this patch if you like. However I believe
> > that before that one we need Anusha's dmc load patch that is one that I'm 
> > also
> > missing on this series here, right?
> > 
> > So any option works for me and if you decide to move with this feel free to
> > use
> > Reviewed-by: Rodrigo Vivi 
> > 
> > And please let me know what are your plans with this runtime_pm related
> > patches and if you need anything from me.
> 
> I was planning to do those as a follow up once this series is merged. I didn't
> want to make it any bigger to make review easier.

yep. Makes sense. Good idea.

> 
> Ander
> 
> > 
> > Thanks,
> > Rodrigo.
> > 
> > On Tue, Nov 29, 2016 at 05:47:12PM +0200, Ander Conselvan de Oliveira wrote:
> > > 
> > > Geminilake is mostly backwards compatible with broxton, so change most
> > > of the IS_BROXTON() checks to IS_GEN9_LP(). Differences between the
> > > platforms will be implemented in follow-up patches.
> > > 
> > > v2: Don't reuse broxton's path in intel_update_max_cdclk().
> > > Don't set plane count as in broxton.
> > > 
> > > v3: Rebase
> > > 
> > > v4: Include the check intel_bios_is_port_hpd_inverted().
> > > Commit message.
> > > 
> > > v5: Leave i915_dmc_info() out; glk's csr version != bxt's. (Rodrigo)
> > > 
> > > v6: Rebase.
> > > 
> > > v7: Convert a few mode IS_BROXTON() occurances in pps, ddi, dsi and pll
> > > code. (Rodrigo)
> > > 
> > > Cc: Rodrigo Vivi 
> > > Signed-off-by: Ander Conselvan de Oliveira 
> > >  > > l.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 10 +-
> > >  drivers/gpu/drm/i915/i915_gem_gtt.c |  8 
> > >  drivers/gpu/drm/i915/i915_irq.c | 10 +-
> > >  drivers/gpu/drm/i915/i915_reg.h |  4 ++--
> > >  drivers/gpu/drm/i915/intel_bios.c   |  2 +-
> > >  drivers/gpu/drm/i915/intel_ddi.c| 18 +-
> > >  drivers/gpu/drm/i915/intel_display.c| 12 ++--
> > >  drivers/gpu/drm/i915/intel_dp.c | 24 
> > >  drivers/gpu/drm/i915/intel_dpio_phy.c   |  1 -
> > >  drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
> > >  drivers/gpu/drm/i915/intel_dsi.c| 30 
> > > +++---
> > >  drivers/gpu/drm/i915/intel_dsi_pll.c| 12 ++--
> > >  drivers/gpu/drm/i915/intel_hdmi.c   |  6 +++---
> > >  drivers/gpu/drm/i915/intel_i2c.c|  4 ++--
> > >  drivers/gpu/drm/i915/intel_mocs.c   |  2 +-
> > >  drivers/gpu/drm/i915/intel_panel.c  |  2 +-
> > >  drivers/gpu/drm/i915/intel_pm.c |  6 +++---
> > >  drivers/gpu/drm/i915/intel_runtime_pm.c |  6 +++---
> > >  18 files changed, 79 insertions(+), 80 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 8eb8c29..6c17d39 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -1108,7 +1108,7 @@ static int i915_frequency_info(struct seq_file *m,
> > > void *unused)
> > >   int max_freq;
> > > 
> > >   rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
> > > - if (IS_BROXTON(dev_priv)) {
> > > + if (IS_GEN9_LP(dev_priv)) {
> > >   rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
> > >   gt_perf_status = I915_READ(BXT_GT_PERF_STATUS);
> > >   } else {
> > > @@ -1204,7 +1204,7 @@ static int i915_frequency_info(struct seq_file *m,
> > > void *unused)
> > >   seq_printf(m, "Down threshold: %d%%\n",
> > >  dev_priv->rps.down_threshold);
> > > 
> > > - max_freq = (IS_BROXTON(dev_priv) ? rp_state_cap >> 0 :
> > > + max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 0 :
> > >   rp_state_cap >> 16) & 0xff;
> > >   max_freq *= (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)
> > > ?
> > >GEN9_FREQ_SCALER : 1);
> > > @@ -1217,7 +1217,7 @@ static int i915_frequency_info(struct seq_file *m,
> > > void *unused)
> > >   seq_printf(m, "Nominal (RP1) frequency: 

Re: [Intel-gfx] [PATCH v3] drm/i915/glk: Reuse broxton code for geminilake

[Ander Conselvan De Oliveira]

On Thu, 2016-12-01 at 17:06 -0800, Rodrigo Vivi wrote:
> A reviewed backwards because I was willing to check if all ifs were in place.
> 
> I missed the ones from i915_drv.c
> 
> *** i915_drv.c:
> i915_drm_suspend_late[1500]fw_csr = !IS_GEN9_LP(dev_priv) &&
> i915_drm_suspend_late[1513]if (IS_GEN9_LP(dev_priv))
> i915_drm_resume_early[1721]if (IS_GEN9_LP(dev_priv)) {
> i915_drm_resume_early[1731]if (IS_GEN9_LP(dev_priv) ||
> intel_runtime_suspend[2354]if (IS_GEN9_LP(dev_priv)) {
> intel_runtime_resume[2439] if (IS_GEN9_LP(dev_priv)) {
> 
> The ones that I had added with the patch
> "drm/i915/glk: Add missing bits to allow runtime pm suspend on GLK."
> 
> Feel free to squash all those into this patch if you like. However I believe
> that before that one we need Anusha's dmc load patch that is one that I'm also
> missing on this series here, right?
> 
> So any option works for me and if you decide to move with this feel free to
> use
> Reviewed-by: Rodrigo Vivi 
> 
> And please let me know what are your plans with this runtime_pm related
> patches and if you need anything from me.

I was planning to do those as a follow up once this series is merged. I didn't
want to make it any bigger to make review easier.

Ander

> 
> Thanks,
> Rodrigo.
> 
> On Tue, Nov 29, 2016 at 05:47:12PM +0200, Ander Conselvan de Oliveira wrote:
> > 
> > Geminilake is mostly backwards compatible with broxton, so change most
> > of the IS_BROXTON() checks to IS_GEN9_LP(). Differences between the
> > platforms will be implemented in follow-up patches.
> > 
> > v2: Don't reuse broxton's path in intel_update_max_cdclk().
> > Don't set plane count as in broxton.
> > 
> > v3: Rebase
> > 
> > v4: Include the check intel_bios_is_port_hpd_inverted().
> > Commit message.
> > 
> > v5: Leave i915_dmc_info() out; glk's csr version != bxt's. (Rodrigo)
> > 
> > v6: Rebase.
> > 
> > v7: Convert a few mode IS_BROXTON() occurances in pps, ddi, dsi and pll
> > code. (Rodrigo)
> > 
> > Cc: Rodrigo Vivi 
> > Signed-off-by: Ander Conselvan de Oliveira  > l.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 10 +-
> >  drivers/gpu/drm/i915/i915_gem_gtt.c |  8 
> >  drivers/gpu/drm/i915/i915_irq.c | 10 +-
> >  drivers/gpu/drm/i915/i915_reg.h |  4 ++--
> >  drivers/gpu/drm/i915/intel_bios.c   |  2 +-
> >  drivers/gpu/drm/i915/intel_ddi.c| 18 +-
> >  drivers/gpu/drm/i915/intel_display.c| 12 ++--
> >  drivers/gpu/drm/i915/intel_dp.c | 24 
> >  drivers/gpu/drm/i915/intel_dpio_phy.c   |  1 -
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
> >  drivers/gpu/drm/i915/intel_dsi.c| 30 +++---
> >  drivers/gpu/drm/i915/intel_dsi_pll.c| 12 ++--
> >  drivers/gpu/drm/i915/intel_hdmi.c   |  6 +++---
> >  drivers/gpu/drm/i915/intel_i2c.c|  4 ++--
> >  drivers/gpu/drm/i915/intel_mocs.c   |  2 +-
> >  drivers/gpu/drm/i915/intel_panel.c  |  2 +-
> >  drivers/gpu/drm/i915/intel_pm.c |  6 +++---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c |  6 +++---
> >  18 files changed, 79 insertions(+), 80 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 8eb8c29..6c17d39 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1108,7 +1108,7 @@ static int i915_frequency_info(struct seq_file *m,
> > void *unused)
> >     int max_freq;
> > 
> >     rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
> > -   if (IS_BROXTON(dev_priv)) {
> > +   if (IS_GEN9_LP(dev_priv)) {
> >     rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
> >     gt_perf_status = I915_READ(BXT_GT_PERF_STATUS);
> >     } else {
> > @@ -1204,7 +1204,7 @@ static int i915_frequency_info(struct seq_file *m,
> > void *unused)
> >     seq_printf(m, "Down threshold: %d%%\n",
> >        dev_priv->rps.down_threshold);
> > 
> > -   max_freq = (IS_BROXTON(dev_priv) ? rp_state_cap >> 0 :
> > +   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 0 :
> >     rp_state_cap >> 16) & 0xff;
> >     max_freq *= (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)
> > ?
> >      GEN9_FREQ_SCALER : 1);
> > @@ -1217,7 +1217,7 @@ static int i915_frequency_info(struct seq_file *m,
> > void *unused)
> >     seq_printf(m, "Nominal (RP1) frequency: %dMHz\n",
> >        intel_gpu_freq(dev_priv, max_freq));
> > 
> > -   max_freq = (IS_BROXTON(dev_priv) ? rp_state_cap >> 16 :
> > +   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 16 :
> >     rp_state_cap >> 0) & 0xff;
> >     max_freq *= 

Re: [Intel-gfx] [PATCH v3] drm/i915/glk: Reuse broxton code for geminilake

[Rodrigo Vivi]

A reviewed backwards because I was willing to check if all ifs were in place.

I missed the ones from i915_drv.c

*** i915_drv.c:
i915_drm_suspend_late[1500]fw_csr = !IS_GEN9_LP(dev_priv) &&
i915_drm_suspend_late[1513]if (IS_GEN9_LP(dev_priv))
i915_drm_resume_early[1721]if (IS_GEN9_LP(dev_priv)) {
i915_drm_resume_early[1731]if (IS_GEN9_LP(dev_priv) ||
intel_runtime_suspend[2354]if (IS_GEN9_LP(dev_priv)) {
intel_runtime_resume[2439] if (IS_GEN9_LP(dev_priv)) {

The ones that I had added with the patch
"drm/i915/glk: Add missing bits to allow runtime pm suspend on GLK."

Feel free to squash all those into this patch if you like. However I believe 
that before that one we need Anusha's dmc load patch that is one that I'm also 
missing on this series here, right?

So any option works for me and if you decide to move with this feel free to use
Reviewed-by: Rodrigo Vivi 

And please let me know what are your plans with this runtime_pm related patches 
and if you need anything from me.

Thanks,
Rodrigo.

On Tue, Nov 29, 2016 at 05:47:12PM +0200, Ander Conselvan de Oliveira wrote:
> Geminilake is mostly backwards compatible with broxton, so change most
> of the IS_BROXTON() checks to IS_GEN9_LP(). Differences between the
> platforms will be implemented in follow-up patches.
>
> v2: Don't reuse broxton's path in intel_update_max_cdclk().
> Don't set plane count as in broxton.
>
> v3: Rebase
>
> v4: Include the check intel_bios_is_port_hpd_inverted().
> Commit message.
>
> v5: Leave i915_dmc_info() out; glk's csr version != bxt's. (Rodrigo)
>
> v6: Rebase.
>
> v7: Convert a few mode IS_BROXTON() occurances in pps, ddi, dsi and pll
> code. (Rodrigo)
>
> Cc: Rodrigo Vivi 
> Signed-off-by: Ander Conselvan de Oliveira 
> 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 10 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c |  8 
>  drivers/gpu/drm/i915/i915_irq.c | 10 +-
>  drivers/gpu/drm/i915/i915_reg.h |  4 ++--
>  drivers/gpu/drm/i915/intel_bios.c   |  2 +-
>  drivers/gpu/drm/i915/intel_ddi.c| 18 +-
>  drivers/gpu/drm/i915/intel_display.c| 12 ++--
>  drivers/gpu/drm/i915/intel_dp.c | 24 
>  drivers/gpu/drm/i915/intel_dpio_phy.c   |  1 -
>  drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
>  drivers/gpu/drm/i915/intel_dsi.c| 30 +++---
>  drivers/gpu/drm/i915/intel_dsi_pll.c| 12 ++--
>  drivers/gpu/drm/i915/intel_hdmi.c   |  6 +++---
>  drivers/gpu/drm/i915/intel_i2c.c|  4 ++--
>  drivers/gpu/drm/i915/intel_mocs.c   |  2 +-
>  drivers/gpu/drm/i915/intel_panel.c  |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c |  6 +++---
>  drivers/gpu/drm/i915/intel_runtime_pm.c |  6 +++---
>  18 files changed, 79 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 8eb8c29..6c17d39 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1108,7 +1108,7 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>   int max_freq;
>
>   rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
> - if (IS_BROXTON(dev_priv)) {
> + if (IS_GEN9_LP(dev_priv)) {
>   rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
>   gt_perf_status = I915_READ(BXT_GT_PERF_STATUS);
>   } else {
> @@ -1204,7 +1204,7 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>   seq_printf(m, "Down threshold: %d%%\n",
>  dev_priv->rps.down_threshold);
>
> - max_freq = (IS_BROXTON(dev_priv) ? rp_state_cap >> 0 :
> + max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 0 :
>   rp_state_cap >> 16) & 0xff;
>   max_freq *= (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv) ?
>GEN9_FREQ_SCALER : 1);
> @@ -1217,7 +1217,7 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>   seq_printf(m, "Nominal (RP1) frequency: %dMHz\n",
>  intel_gpu_freq(dev_priv, max_freq));
>
> - max_freq = (IS_BROXTON(dev_priv) ? rp_state_cap >> 16 :
> + max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 16 :
>   rp_state_cap >> 0) & 0xff;
>   max_freq *= (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv) ?
>GEN9_FREQ_SCALER : 1);
> @@ -5169,7 +5169,7 @@ static void gen9_sseu_device_status(struct 
> drm_i915_private *dev_priv,
>   u32 s_reg[s_max], eu_reg[2*s_max], eu_mask[2];
>
>   /* BXT has a single slice and at most 3 subslices. */
> - if (IS_BROXTON(dev_priv)) {
> + if (IS_GEN9_LP(dev_priv)) {
> 

[Intel-gfx] [PATCH v3] drm/i915/glk: Reuse broxton code for geminilake

[Ander Conselvan de Oliveira]

Geminilake is mostly backwards compatible with broxton, so change most
of the IS_BROXTON() checks to IS_GEN9_LP(). Differences between the
platforms will be implemented in follow-up patches.

v2: Don't reuse broxton's path in intel_update_max_cdclk().
Don't set plane count as in broxton.

v3: Rebase

v4: Include the check intel_bios_is_port_hpd_inverted().
Commit message.

v5: Leave i915_dmc_info() out; glk's csr version != bxt's. (Rodrigo)

v6: Rebase.

v7: Convert a few mode IS_BROXTON() occurances in pps, ddi, dsi and pll
code. (Rodrigo)

Cc: Rodrigo Vivi 
Signed-off-by: Ander Conselvan de Oliveira 

---
 drivers/gpu/drm/i915/i915_debugfs.c | 10 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c |  8 
 drivers/gpu/drm/i915/i915_irq.c | 10 +-
 drivers/gpu/drm/i915/i915_reg.h |  4 ++--
 drivers/gpu/drm/i915/intel_bios.c   |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c| 18 +-
 drivers/gpu/drm/i915/intel_display.c| 12 ++--
 drivers/gpu/drm/i915/intel_dp.c | 24 
 drivers/gpu/drm/i915/intel_dpio_phy.c   |  1 -
 drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
 drivers/gpu/drm/i915/intel_dsi.c| 30 +++---
 drivers/gpu/drm/i915/intel_dsi_pll.c| 12 ++--
 drivers/gpu/drm/i915/intel_hdmi.c   |  6 +++---
 drivers/gpu/drm/i915/intel_i2c.c|  4 ++--
 drivers/gpu/drm/i915/intel_mocs.c   |  2 +-
 drivers/gpu/drm/i915/intel_panel.c  |  2 +-
 drivers/gpu/drm/i915/intel_pm.c |  6 +++---
 drivers/gpu/drm/i915/intel_runtime_pm.c |  6 +++---
 18 files changed, 79 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 8eb8c29..6c17d39 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1108,7 +1108,7 @@ static int i915_frequency_info(struct seq_file *m, void 
*unused)
int max_freq;
 
rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
-   if (IS_BROXTON(dev_priv)) {
+   if (IS_GEN9_LP(dev_priv)) {
rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
gt_perf_status = I915_READ(BXT_GT_PERF_STATUS);
} else {
@@ -1204,7 +1204,7 @@ static int i915_frequency_info(struct seq_file *m, void 
*unused)
seq_printf(m, "Down threshold: %d%%\n",
   dev_priv->rps.down_threshold);
 
-   max_freq = (IS_BROXTON(dev_priv) ? rp_state_cap >> 0 :
+   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 0 :
rp_state_cap >> 16) & 0xff;
max_freq *= (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv) ?
 GEN9_FREQ_SCALER : 1);
@@ -1217,7 +1217,7 @@ static int i915_frequency_info(struct seq_file *m, void 
*unused)
seq_printf(m, "Nominal (RP1) frequency: %dMHz\n",
   intel_gpu_freq(dev_priv, max_freq));
 
-   max_freq = (IS_BROXTON(dev_priv) ? rp_state_cap >> 16 :
+   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 16 :
rp_state_cap >> 0) & 0xff;
max_freq *= (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv) ?
 GEN9_FREQ_SCALER : 1);
@@ -5169,7 +5169,7 @@ static void gen9_sseu_device_status(struct 
drm_i915_private *dev_priv,
u32 s_reg[s_max], eu_reg[2*s_max], eu_mask[2];
 
/* BXT has a single slice and at most 3 subslices. */
-   if (IS_BROXTON(dev_priv)) {
+   if (IS_GEN9_LP(dev_priv)) {
s_max = 1;
ss_max = 3;
}
@@ -5203,7 +5203,7 @@ static void gen9_sseu_device_status(struct 
drm_i915_private *dev_priv,
for (ss = 0; ss < ss_max; ss++) {
unsigned int eu_cnt;
 
-   if (IS_BROXTON(dev_priv)) {
+   if (IS_GEN9_LP(dev_priv)) {
if (!(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss
/* skip disabled subslice */
continue;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6cee707..e721a6f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -372,7 +372,7 @@ static void kunmap_page_dma(struct drm_i915_private 
*dev_priv, void *vaddr)
/* There are only few exceptions for gen >=6. chv and bxt.
 * And we are not sure about the latter so play safe for now.
 */
-   if (IS_CHERRYVIEW(dev_priv) || IS_BROXTON(dev_priv))
+   if (IS_CHERRYVIEW(dev_priv) || IS_GEN9_LP(dev_priv))
drm_clflush_virt_range(vaddr, PAGE_SIZE);
 
kunmap_atomic(vaddr);
@@ -2940,7 +2940,7 @@ static