diff mbox series

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

Message ID 20180726001229.13791-1-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/icl: don't set CNL_DDI_CLOCk_REG_ACCESS_ON anymore | expand

Commit Message

Zanoni, Paulo R July 26, 2018, 12:12 a.m. UTC
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(-)

Comments

Souza, Jose July 26, 2018, 9:02 p.m. UTC | #1
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)
Zanoni, Paulo R July 27, 2018, 10:06 p.m. UTC | #2
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)
diff mbox series

Patch

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)