[Intel-gfx] [v2] drm/i915/skl: Init cdclk in the driver rather than relying on pre-os
Kumar, Shobhit
shobhit.kumar at linux.intel.com
Fri Oct 16 06:08:01 PDT 2015
On 10/08/2015 05:54 PM, Ville Syrjälä wrote:
> On Thu, Oct 08, 2015 at 05:43:41PM +0530, Kumar, Shobhit wrote:
>> On 10/08/2015 04:59 PM, Imre Deak wrote:
>>> On to, 2015-10-08 at 09:58 +0530, Shobhit Kumar wrote:
>>>> Reuse what is programmed by pre-os, but in case there is no pre-os
>>>> initialization, init the cdclk with the max value by default untill
>>>> dynamic cdclk support comes.
>>>>
>>>> v2: Check if BIOS programmed correctly rather than always calling init
>>>> - Do validation of programmed cdctl and what it is expected
>>>> - Only do slk_init_cdclk if validation failed else reuse BIOS
>>>> programmed value
>>>>
>>>> Cc: Imre Deak <imre.deak at intel.com>
>>>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>> Signed-off-by: Shobhit Kumar <shobhit.kumar at intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/intel_ddi.c | 18 ++++++++++++-----
>>>> drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++---------
>>>> 2 files changed, 42 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>>> index 2d3cc82..3ec5618 100644
>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>>> @@ -2946,11 +2946,19 @@ void intel_ddi_pll_init(struct drm_device *dev)
>>>> int cdclk_freq;
>>>>
>>>> cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
>>>> - dev_priv->skl_boot_cdclk = cdclk_freq;
>>>> - if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
>>>> - DRM_ERROR("LCPLL1 is disabled\n");
>>>> - else
>>>> - intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
>>>> +
>>>> + /* Invalid CDCLK from BIOS ? */
>>>> + if (cdclk_freq < 0) {
>>>> + /* program to maximum cdclk till we have dynamic cdclk support */
>>>> + dev_priv->skl_boot_cdclk = 675000;
>>>> + skl_init_cdclk(dev_priv);
>>>
>>> This would still try to reprogram CDCLK if BIOS enabled an output with
>>> an incorrect CDCLK decimal part. Isn't this the exact scenario you're
>>> seeing? As said before reprogramming CDCLK in that case would require
>>> disabling the outputs first.
>>
>> I went with the hypothesis that if VBIOS/GOP was loaded it would have to
>> correct the cdclock, with wrong decimal value display cannot be enabled.
>> For example AUX will fail on SKL. So for correct display output enabled
>> cdclock has to be correctly programmed. If it is wrong display was not
>> enabled at all.
>>
>> The scenario which I am seeing is VBIOS/GOP is not loaded at all, and
>> the pre-os is not leaving cdclock in correct state.
>
> But it still enabled DPLL0? Why does it do that if it doesn't set up
> cdclk?
>
Okay digging more into FSP code, found a bug where even when display is
not initialized in pre-os, still there was an initialization path for
enabling display audio for HDMI which was enabling DPLL0. That also
should have been not invoked when display itself is not being loaded.
Not sure what going on in there !! Will work with FSP team and get this
rectified. After that sanitize patch(following up this mail) can check
the clock related programming and sanitize if needed as discussed with
you Ville on IRC.
Regards
Shobhit
>>
>>>
>>> We would also need to call skl_init_cdclk if the BIOS hasn't enabled the
>>> PLL for some reason. But I guess that could be a separate change.
>>>
>>>> + } else {
>>>> + dev_priv->skl_boot_cdclk = cdclk_freq;
>>>> + if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
>>>> + DRM_ERROR("LCPLL1 is disabled\n");
>>>> + else
>>>> + intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
>>>> + }
>>>> } else if (IS_BROXTON(dev)) {
>>>> broxton_init_cdclk(dev);
>>>> broxton_ddi_phy_init(dev);
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index bbeb6d3..f734410 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -6634,12 +6634,15 @@ static int skylake_get_display_clock_speed(struct drm_device *dev)
>>>> uint32_t lcpll1 = I915_READ(LCPLL1_CTL);
>>>> uint32_t cdctl = I915_READ(CDCLK_CTL);
>>>> uint32_t linkrate;
>>>> + int freq;
>>>>
>>>> if (!(lcpll1 & LCPLL_PLL_ENABLE))
>>>> return 24000; /* 24MHz is the cd freq with NSSC ref */
>>>>
>>>> - if ((cdctl & CDCLK_FREQ_SEL_MASK) == CDCLK_FREQ_540)
>>>> - return 540000;
>>>> + if ((cdctl & CDCLK_FREQ_SEL_MASK) == CDCLK_FREQ_540) {
>>>> + freq = 540000;
>>>> + goto verify;
>>>> + }
>>>>
>>>> linkrate = (I915_READ(DPLL_CTRL1) &
>>>> DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
>>>> @@ -6649,30 +6652,46 @@ static int skylake_get_display_clock_speed(struct drm_device *dev)
>>>> /* vco 8640 */
>>>> switch (cdctl & CDCLK_FREQ_SEL_MASK) {
>>>> case CDCLK_FREQ_450_432:
>>>> - return 432000;
>>>> + freq = 432000;
>>>> + break;
>>>> case CDCLK_FREQ_337_308:
>>>> - return 308570;
>>>> + freq = 308570;
>>>> + break;
>>>> case CDCLK_FREQ_675_617:
>>>> - return 617140;
>>>> + freq = 617140;
>>>> + break;
>>>> default:
>>>> WARN(1, "Unknown cd freq selection\n");
>>>> + return -EINVAL;
>>>> }
>>>> } else {
>>>> /* vco 8100 */
>>>> switch (cdctl & CDCLK_FREQ_SEL_MASK) {
>>>> case CDCLK_FREQ_450_432:
>>>> - return 450000;
>>>> + freq = 450000;
>>>> + break;
>>>> case CDCLK_FREQ_337_308:
>>>> - return 337500;
>>>> + freq = 337500;
>>>> + break;
>>>> case CDCLK_FREQ_675_617:
>>>> - return 675000;
>>>> + freq = 675000;
>>>> + break;
>>>> default:
>>>> WARN(1, "Unknown cd freq selection\n");
>>>> + return -EINVAL;
>>>> }
>>>> }
>>>>
>>>> - /* error case, do as if DPLL0 isn't enabled */
>>>> - return 24000;
>>>> +verify:
>>>> + /*
>>>> + * Noticed in some instances that the freq selection is correct but
>>>> + * decimal part is programmed wrong from BIOS where pre-os does not
>>>> + * enable display. Verify the same as well.
>>>> + */
>>>> + if (cdctl == ((cdctl & CDCLK_FREQ_SEL_MASK) | skl_cdclk_decimal(freq)))
>>>> + return freq;
>>>> + else
>>>> + return -EINVAL;
>>>> }
>>>>
>>>> static int broxton_get_display_clock_speed(struct drm_device *dev)
>>>
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
More information about the Intel-gfx
mailing list