[PATCH] drm/i915/icl: don't set CNL_DDI_CLOCk_REG_ACCESS_ON anymore

[PATCH] drm/i915/icl: don't set CNL_DDI_CLOCk_REG_ACCESS_ON anymore

[Paulo Zanoni]

The new recommendation from the spec is to simply not set this bit
anymore. Not setting the bit would prevent some hangs that our driver
manages to avoid since commit c8af5274c3cb ("drm/i915: enable the
pipe/transcoder/planes later on HSW+"), and the theoretical downside
of not setting the bit doesn't seem realistic according to the HW
team. Let's follow their recommendation.

BSpec: 20233
References: commit c8af5274c3cb ("drm/i915: enable the
 pipe/transcoder/planes later on HSW+")
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6b5aa3b074ec..cf89141b2281 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3372,10 +3372,6 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
 
 	/* 7. Setup MBUS. */
 	icl_mbus_init(dev_priv);
-
-	/* 8. CHICKEN_DCPR_1 */
-	I915_WRITE(GEN8_CHICKEN_DCPR_1, I915_READ(GEN8_CHICKEN_DCPR_1) |
-					CNL_DDI_CLOCK_REG_ACCESS_ON);
 }
 
 static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
-- 
2.14.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915/icl: don't set CNL_DDI_CLOCk_REG_ACCESS_ON anymore

[Patchwork]

== Series Details ==

Series: drm/i915/icl: don't set CNL_DDI_CLOCk_REG_ACCESS_ON anymore
URL   : https://patchwork.freedesktop.org/series/47259/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4547 -> Patchwork_9771 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9771 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9771, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47259/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9771:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-reload-inject:
      fi-bxt-j4205:       PASS -> DMESG-WARN

    igt@drv_selftest@live_objects:
      fi-cnl-psr:         NOTRUN -> DMESG-FAIL

    
== Known issues ==

  Here are the changes found in Patchwork_9771 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-no-display:
      fi-cnl-psr:         NOTRUN -> DMESG-WARN (fdo#105395) +6

    igt@gem_exec_suspend@basic-s3:
      {fi-skl-caroline}:  NOTRUN -> INCOMPLETE (fdo#104108)

    igt@kms_flip@basic-flip-vs-dpms:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106000)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-FAIL (fdo#102614, fdo#106103)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)

    {igt@kms_psr@cursor_plane_move}:
      fi-cnl-psr:         NOTRUN -> DMESG-FAIL (fdo#107372) +1

    
    ==== Possible fixes ====

    igt@gem_exec_create@basic:
      fi-glk-j4005:       DMESG-WARN (fdo#106745) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       DMESG-WARN (fdo#106000) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#105395 https://bugs.freedesktop.org/show_bug.cgi?id=105395
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
  fdo#106745 https://bugs.freedesktop.org/show_bug.cgi?id=106745
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (48 -> 45) ==

  Additional (3): fi-byt-j1900 fi-skl-caroline fi-cnl-psr 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper 


== Build changes ==

    * Linux: CI_DRM_4547 -> Patchwork_9771

  CI_DRM_4547: 0a7ab192a697e951b2404f3c1ce42c5fa74f9ed1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4575: fe908a01012c9daafafb3410b9407725ca9d4f21 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9771: 31f5476571313e956468cfbe65af4d88ed554448 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

31f547657131 drm/i915/icl: don't set CNL_DDI_CLOCk_REG_ACCESS_ON anymore

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9771/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/icl: don't set CNL_DDI_CLOCk_REG_ACCESS_ON anymore

[Souza, Jose]

On Wed, 2018-07-25 at 17:12 -0700, Paulo Zanoni wrote:
> The new recommendation from the spec is to simply not set this bit
> anymore. Not setting the bit would prevent some hangs that our driver
> manages to avoid since commit c8af5274c3cb ("drm/i915: enable the
> pipe/transcoder/planes later on HSW+"), and the theoretical downside
> of not setting the bit doesn't seem realistic according to the HW
> team. Let's follow their recommendation.
> 
> BSpec: 20233
> References: commit c8af5274c3cb ("drm/i915: enable the
>  pipe/transcoder/planes later on HSW+")
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Also another steps were removed from initialization too, the previous
step 4 and parts of step 3 but if we really need to remove it we can do
it another patch.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6b5aa3b074ec..cf89141b2281 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3372,10 +3372,6 @@ static void icl_display_core_init(struct
> drm_i915_private *dev_priv,
>  
>  	/* 7. Setup MBUS. */
>  	icl_mbus_init(dev_priv);
> -
> -	/* 8. CHICKEN_DCPR_1 */
> -	I915_WRITE(GEN8_CHICKEN_DCPR_1, I915_READ(GEN8_CHICKEN_DCPR_1)
> |
> -					CNL_DDI_CLOCK_REG_ACCESS_ON);
>  }
>  
>  static void icl_display_core_uninit(struct drm_i915_private
> *dev_priv)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/icl: don't set CNL_DDI_CLOCk_REG_ACCESS_ON anymore

[Paulo Zanoni]

Em Qui, 2018-07-26 às 14:02 -0700, Souza, Jose escreveu:
> On Wed, 2018-07-25 at 17:12 -0700, Paulo Zanoni wrote:
> > The new recommendation from the spec is to simply not set this bit
> > anymore. Not setting the bit would prevent some hangs that our
> > driver
> > manages to avoid since commit c8af5274c3cb ("drm/i915: enable the
> > pipe/transcoder/planes later on HSW+"), and the theoretical
> > downside
> > of not setting the bit doesn't seem realistic according to the HW
> > team. Let's follow their recommendation.
> > 
> > BSpec: 20233
> > References: commit c8af5274c3cb ("drm/i915: enable the
> >  pipe/transcoder/planes later on HSW+")
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Also another steps were removed from initialization too, the previous
> step 4 and parts of step 3 but if we really need to remove it we can
> do
> it another patch.

What I see is that what our code lists as Step 3 was moved on the spec
to be part of step 2. I can't find anything we should remove.

What I see that we may be missing is the text about initializing the
PCH clocks (step 1.b). We should probably do something about it.

> 
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

Thanks!

> 
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 6b5aa3b074ec..cf89141b2281 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3372,10 +3372,6 @@ static void icl_display_core_init(struct
> > drm_i915_private *dev_priv,
> >  
> >  	/* 7. Setup MBUS. */
> >  	icl_mbus_init(dev_priv);
> > -
> > -	/* 8. CHICKEN_DCPR_1 */
> > -	I915_WRITE(GEN8_CHICKEN_DCPR_1,
> > I915_READ(GEN8_CHICKEN_DCPR_1)
> > > 
> > 
> > -					CNL_DDI_CLOCK_REG_ACCESS_O
> > N);
> >  }
> >  
> >  static void icl_display_core_uninit(struct drm_i915_private
> > *dev_priv)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx