[Intel-gfx] [PATCH 2/2] drm/i915: fix wrong PLL debug messages.

[Paulo Zanoni]

2014/1/8 Damien Lespiau <damien.lespiau at intel.com>:
> On Wed, Jan 08, 2014 at 11:12:28AM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> LPT does have PCH refclk, but it's different form the IBX/CPT/PPT one
>> and doesn't use the same structs. It is wrong to have a message saying
>> that "LPT does not has PCH refclk" (sic). While at it, signal that we
>> only want this function on IBX/CPT/PPT by renaming it and adding a
>> WARN.
>>
>> On HSW we also print "0 shared PLLs initialized", but we *do* have
>> shared PLLs on HSW (LCPLL, WRPLL, SPLL) and we *do* initialize them.
>> We just don't use "struct intel_shared_dpll". So remove the debug
>> message.
>>
>> In the future we may want to rename all that "intel shared pll" code
>> to "ibx shared pll", but I'll leave this to another patch.
>
> I remember Daniel saying that his plan was to make the DDI clocks use
> the shared PLL struct. I guess that'd take care of the debug message
> correctness.

Yes, but there's currently no plan actually do that conversion, so
we'll stay with wrong messages for an undefined amount of time. The
first message I removed is not even ever print, and the second message
always prints "2" for IBX/CPT/PPT and 0 for LPT+: you can easily check
that by looking at the code, so the messages are not even useful.

Thanks for the reviews!
Paulo

>
> So I defer this one to higher authorities, you have my r-b tag as the
> patch makes sense, not sure if Daniel will take it though :)
>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 11 +++--------
>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 034952c..df3cf69 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -1211,15 +1211,12 @@ static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
>>       }
>>  }
>>
>> -static void assert_pch_refclk_enabled(struct drm_i915_private *dev_priv)
>> +static void ibx_assert_pch_refclk_enabled(struct drm_i915_private *dev_priv)
>>  {
>>       u32 val;
>>       bool enabled;
>>
>> -     if (HAS_PCH_LPT(dev_priv->dev)) {
>> -             DRM_DEBUG_DRIVER("LPT does not has PCH refclk, skipping check\n");
>> -             return;
>> -     }
>> +     WARN_ON(!(HAS_PCH_IBX(dev_priv->dev) || HAS_PCH_CPT(dev_priv->dev)));
>>
>>       val = I915_READ(PCH_DREF_CONTROL);
>>       enabled = !!(val & (DREF_SSC_SOURCE_MASK | DREF_NONSPREAD_SOURCE_MASK |
>> @@ -10067,7 +10064,7 @@ static void ibx_pch_dpll_enable(struct drm_i915_private *dev_priv,
>>                               struct intel_shared_dpll *pll)
>>  {
>>       /* PCH refclock must be enabled first */
>> -     assert_pch_refclk_enabled(dev_priv);
>> +     ibx_assert_pch_refclk_enabled(dev_priv);
>>
>>       I915_WRITE(PCH_DPLL(pll->id), pll->hw_state.dpll);
>>
>> @@ -10135,8 +10132,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
>>               dev_priv->num_shared_dpll = 0;
>>
>>       BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
>> -     DRM_DEBUG_KMS("%i shared PLLs initialized\n",
>> -                   dev_priv->num_shared_dpll);
>>  }
>>
>>  static void intel_crtc_init(struct drm_device *dev, int pipe)
>> --
>> 1.8.4.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni