| Message ID | 1522960670-29948-1-git-send-email-abhay.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 05 Apr 2018, Abhay Kumar <abhay.kumar@intel.com> wrote: > In glk when device boots with 1366x768 panel, HDA codec doesn't comeup. > This result in no audio forever as cdclk is < 96Mhz. > This chagne will ensure CD clock to be twice of BCLK. In short, we can't poke around CDCLK like this. It needs a full modeset, and it's non-trivial from the path you're calling this from. I'm considering pushing the original patch [1], because we haven't come up with working alternatives. Please confirm that the patch reliably fixes the issue. (Though I think if you boot with *all* outputs disabled, we'll choose the lowest CDCLK possible regardless of the patch, reproducing the same issue.) What's the CDCLK frequency set by the BIOS/GOP at boot? There are no logs with drm.debug=14 attached to the referenced bug. I see that bspec says, "158.4 MHz CD (Display Audio enumeration and playback OK)" but that's *not* twice the BCLK. I'm inclined to lean towards 192 MHz min leading to max CDCLK on GLK. BR, Jani. [1] https://patchwork.freedesktop.org/patch/184778/ > > v2: > - Address comment (Jani) > - New design approach > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937 > Signed-off-by: Abhay Kumar <abhay.kumar@intel.com> > --- > drivers/gpu/drm/i915/intel_audio.c | 33 ++++++++++++++++++++++----------- > drivers/gpu/drm/i915/intel_cdclk.c | 21 +++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > 3 files changed, 44 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > index 709d6ca68074..f7dd3d532e93 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -723,15 +723,37 @@ static void i915_audio_component_put_power(struct device *kdev) > intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO); > } > > +/* Get CDCLK in kHz */ > +static int i915_audio_component_get_cdclk_freq(struct device *kdev) > +{ > + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > + > + if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) > + return -ENODEV; > + > + return dev_priv->cdclk.hw.cdclk; > +} > + > static void i915_audio_component_codec_wake_override(struct device *kdev, > bool enable) > { > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > u32 tmp; > + int current_cdclk; > > if (!IS_GEN9_BC(dev_priv)) > return; > > + current_cdclk = i915_audio_component_get_cdclk_freq(kdev); > + > + /* > + * Before probing for HDA Codec we need to make sure > + * "The CD clock frequency must be at least twice > + * the frequency of the Azalia BCLK." > + */ > + if (INTEL_GEN(dev_priv) >= 9 && current_cdclk <= 192000) > + intel_cdclk_bump(dev_priv); > + > i915_audio_component_get_power(kdev); > > /* > @@ -753,17 +775,6 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, > i915_audio_component_put_power(kdev); > } > > -/* Get CDCLK in kHz */ > -static int i915_audio_component_get_cdclk_freq(struct device *kdev) > -{ > - struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > - > - if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) > - return -ENODEV; > - > - return dev_priv->cdclk.hw.cdclk; > -} > - > /* > * get the intel_encoder according to the parameter port and pipe > * intel_encoder is saved by the index of pipe > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index dc7db8a2caf8..9426e1b7badc 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1516,6 +1516,27 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv) > } > > /** > + * intel_cdclk_bump - Increase cdclk to 2* BCLK > + * @dev_priv: i915 device > + * > + * Increase CDCLK for GKL and CNL. This is done only > + * during HDA codec probe. > + */ > +void intel_cdclk_bump(struct drm_i915_private *dev_priv) > +{ > + struct intel_cdclk_state cdclk_state; > + > + cdclk_state = dev_priv->cdclk.hw; > + > + if (IS_GEMINILAKE(dev_priv)) { > + cdclk_state.cdclk = glk_calc_cdclk((2*96000)); > + cdclk_state.vco = glk_de_pll_vco(dev_priv, cdclk_state.cdclk); > + cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk); > + bxt_set_cdclk(dev_priv, &cdclk_state); > + } > +} > + > +/** > * bxt_uninit_cdclk - Uninitialize CDCLK on BXT > * @dev_priv: i915 device > * > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index d1452fd2a58d..5192753df3dc 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1417,6 +1417,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv); > void cnl_init_cdclk(struct drm_i915_private *dev_priv); > void cnl_uninit_cdclk(struct drm_i915_private *dev_priv); > void bxt_init_cdclk(struct drm_i915_private *dev_priv); > +void intel_cdclk_bump(struct drm_i915_private *dev_priv); > void bxt_uninit_cdclk(struct drm_i915_private *dev_priv); > void icl_init_cdclk(struct drm_i915_private *dev_priv); > void icl_uninit_cdclk(struct drm_i915_private *dev_priv);
On Fri, Apr 06, 2018 at 04:47:08PM +0300, Jani Nikula wrote: > On Thu, 05 Apr 2018, Abhay Kumar <abhay.kumar@intel.com> wrote: > > In glk when device boots with 1366x768 panel, HDA codec doesn't comeup. > > This result in no audio forever as cdclk is < 96Mhz. > > This chagne will ensure CD clock to be twice of BCLK. > > In short, we can't poke around CDCLK like this. It needs a full modeset, > and it's non-trivial from the path you're calling this from. I tried to cobble something together quickly: git://github.com/vsyrjala/linux.git glk_cnl_cdclk_audio Not tested at all, and I have no idea if my assumptions about snd_hda_intel actually hold. > > I'm considering pushing the original patch [1], because we haven't come > up with working alternatives. Please confirm that the patch reliably > fixes the issue. > > (Though I think if you boot with *all* outputs disabled, we'll choose > the lowest CDCLK possible regardless of the patch, reproducing the same > issue.) > > What's the CDCLK frequency set by the BIOS/GOP at boot? There are no > logs with drm.debug=14 attached to the referenced bug. > > I see that bspec says, "158.4 MHz CD (Display Audio enumeration and > playback OK)" but that's *not* twice the BCLK. I'm inclined to lean > towards 192 MHz min leading to max CDCLK on GLK. > > BR, > Jani. > > > [1] https://patchwork.freedesktop.org/patch/184778/ > > > > > > > v2: > > - Address comment (Jani) > > - New design approach > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937 > > Signed-off-by: Abhay Kumar <abhay.kumar@intel.com> > > --- > > drivers/gpu/drm/i915/intel_audio.c | 33 ++++++++++++++++++++++----------- > > drivers/gpu/drm/i915/intel_cdclk.c | 21 +++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > 3 files changed, 44 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > > index 709d6ca68074..f7dd3d532e93 100644 > > --- a/drivers/gpu/drm/i915/intel_audio.c > > +++ b/drivers/gpu/drm/i915/intel_audio.c > > @@ -723,15 +723,37 @@ static void i915_audio_component_put_power(struct device *kdev) > > intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO); > > } > > > > +/* Get CDCLK in kHz */ > > +static int i915_audio_component_get_cdclk_freq(struct device *kdev) > > +{ > > + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > + > > + if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) > > + return -ENODEV; > > + > > + return dev_priv->cdclk.hw.cdclk; > > +} > > + > > static void i915_audio_component_codec_wake_override(struct device *kdev, > > bool enable) > > { > > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > u32 tmp; > > + int current_cdclk; > > > > if (!IS_GEN9_BC(dev_priv)) > > return; > > > > + current_cdclk = i915_audio_component_get_cdclk_freq(kdev); > > + > > + /* > > + * Before probing for HDA Codec we need to make sure > > + * "The CD clock frequency must be at least twice > > + * the frequency of the Azalia BCLK." > > + */ > > + if (INTEL_GEN(dev_priv) >= 9 && current_cdclk <= 192000) > > + intel_cdclk_bump(dev_priv); > > + > > i915_audio_component_get_power(kdev); > > > > /* > > @@ -753,17 +775,6 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, > > i915_audio_component_put_power(kdev); > > } > > > > -/* Get CDCLK in kHz */ > > -static int i915_audio_component_get_cdclk_freq(struct device *kdev) > > -{ > > - struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > - > > - if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) > > - return -ENODEV; > > - > > - return dev_priv->cdclk.hw.cdclk; > > -} > > - > > /* > > * get the intel_encoder according to the parameter port and pipe > > * intel_encoder is saved by the index of pipe > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > > index dc7db8a2caf8..9426e1b7badc 100644 > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > @@ -1516,6 +1516,27 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv) > > } > > > > /** > > + * intel_cdclk_bump - Increase cdclk to 2* BCLK > > + * @dev_priv: i915 device > > + * > > + * Increase CDCLK for GKL and CNL. This is done only > > + * during HDA codec probe. > > + */ > > +void intel_cdclk_bump(struct drm_i915_private *dev_priv) > > +{ > > + struct intel_cdclk_state cdclk_state; > > + > > + cdclk_state = dev_priv->cdclk.hw; > > + > > + if (IS_GEMINILAKE(dev_priv)) { > > + cdclk_state.cdclk = glk_calc_cdclk((2*96000)); > > + cdclk_state.vco = glk_de_pll_vco(dev_priv, cdclk_state.cdclk); > > + cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk); > > + bxt_set_cdclk(dev_priv, &cdclk_state); > > + } > > +} > > + > > +/** > > * bxt_uninit_cdclk - Uninitialize CDCLK on BXT > > * @dev_priv: i915 device > > * > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index d1452fd2a58d..5192753df3dc 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1417,6 +1417,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv); > > void cnl_init_cdclk(struct drm_i915_private *dev_priv); > > void cnl_uninit_cdclk(struct drm_i915_private *dev_priv); > > void bxt_init_cdclk(struct drm_i915_private *dev_priv); > > +void intel_cdclk_bump(struct drm_i915_private *dev_priv); > > void bxt_uninit_cdclk(struct drm_i915_private *dev_priv); > > void icl_init_cdclk(struct drm_i915_private *dev_priv); > > void icl_uninit_cdclk(struct drm_i915_private *dev_priv); > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 4/9/2018 3:33 AM, Ville Syrjälä wrote: > On Fri, Apr 06, 2018 at 04:47:08PM +0300, Jani Nikula wrote: >> On Thu, 05 Apr 2018, Abhay Kumar <abhay.kumar@intel.com> wrote: >>> In glk when device boots with 1366x768 panel, HDA codec doesn't comeup. >>> This result in no audio forever as cdclk is < 96Mhz. >>> This chagne will ensure CD clock to be twice of BCLK. >> In short, we can't poke around CDCLK like this. It needs a full modeset, >> and it's non-trivial from the path you're calling this from. > I tried to cobble something together quickly: > git://github.com/vsyrjala/linux.git glk_cnl_cdclk_audio > > Not tested at all, and I have no idea if my assumptions about > snd_hda_intel actually hold. Hi Ville, Tried your patch but as soon as it enters "glk_force_audio_cdclk" the device locks up and reboots. waited for 30-40 times and it always reboot at same place. It never reaches Chrome OS login screen. Thanks. > >> I'm considering pushing the original patch [1], because we haven't come >> up with working alternatives. Please confirm that the patch reliably >> fixes the issue. >> >> (Though I think if you boot with *all* outputs disabled, we'll choose >> the lowest CDCLK possible regardless of the patch, reproducing the same >> issue.) >> >> What's the CDCLK frequency set by the BIOS/GOP at boot? There are no >> logs with drm.debug=14 attached to the referenced bug. >> >> I see that bspec says, "158.4 MHz CD (Display Audio enumeration and >> playback OK)" but that's *not* twice the BCLK. I'm inclined to lean >> towards 192 MHz min leading to max CDCLK on GLK. >> >> BR, >> Jani. Hi Jani, Dynamic cdclk is disabled in BIOS/GOP hence gop makes it to highest clock i.e 316.8. Will attach logs with drm debug enabled in bug. I am also inclined towards 192 which will make max cdclk.. Currently testing all scenario to confirm if patchset 1 fixes all issue or not. There was 4s delay issue during s0ix from audio which i specifically want to test. Thanks. >> >> >> [1] https://patchwork.freedesktop.org/patch/184778/ >> >> >> >>> v2: >>> - Address comment (Jani) >>> - New design approach >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937 >>> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_audio.c | 33 ++++++++++++++++++++++----------- >>> drivers/gpu/drm/i915/intel_cdclk.c | 21 +++++++++++++++++++++ >>> drivers/gpu/drm/i915/intel_drv.h | 1 + >>> 3 files changed, 44 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c >>> index 709d6ca68074..f7dd3d532e93 100644 >>> --- a/drivers/gpu/drm/i915/intel_audio.c >>> +++ b/drivers/gpu/drm/i915/intel_audio.c >>> @@ -723,15 +723,37 @@ static void i915_audio_component_put_power(struct device *kdev) >>> intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO); >>> } >>> >>> +/* Get CDCLK in kHz */ >>> +static int i915_audio_component_get_cdclk_freq(struct device *kdev) >>> +{ >>> + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); >>> + >>> + if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) >>> + return -ENODEV; >>> + >>> + return dev_priv->cdclk.hw.cdclk; >>> +} >>> + >>> static void i915_audio_component_codec_wake_override(struct device *kdev, >>> bool enable) >>> { >>> struct drm_i915_private *dev_priv = kdev_to_i915(kdev); >>> u32 tmp; >>> + int current_cdclk; >>> >>> if (!IS_GEN9_BC(dev_priv)) >>> return; >>> >>> + current_cdclk = i915_audio_component_get_cdclk_freq(kdev); >>> + >>> + /* >>> + * Before probing for HDA Codec we need to make sure >>> + * "The CD clock frequency must be at least twice >>> + * the frequency of the Azalia BCLK." >>> + */ >>> + if (INTEL_GEN(dev_priv) >= 9 && current_cdclk <= 192000) >>> + intel_cdclk_bump(dev_priv); >>> + >>> i915_audio_component_get_power(kdev); >>> >>> /* >>> @@ -753,17 +775,6 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, >>> i915_audio_component_put_power(kdev); >>> } >>> >>> -/* Get CDCLK in kHz */ >>> -static int i915_audio_component_get_cdclk_freq(struct device *kdev) >>> -{ >>> - struct drm_i915_private *dev_priv = kdev_to_i915(kdev); >>> - >>> - if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) >>> - return -ENODEV; >>> - >>> - return dev_priv->cdclk.hw.cdclk; >>> -} >>> - >>> /* >>> * get the intel_encoder according to the parameter port and pipe >>> * intel_encoder is saved by the index of pipe >>> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c >>> index dc7db8a2caf8..9426e1b7badc 100644 >>> --- a/drivers/gpu/drm/i915/intel_cdclk.c >>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c >>> @@ -1516,6 +1516,27 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv) >>> } >>> >>> /** >>> + * intel_cdclk_bump - Increase cdclk to 2* BCLK >>> + * @dev_priv: i915 device >>> + * >>> + * Increase CDCLK for GKL and CNL. This is done only >>> + * during HDA codec probe. >>> + */ >>> +void intel_cdclk_bump(struct drm_i915_private *dev_priv) >>> +{ >>> + struct intel_cdclk_state cdclk_state; >>> + >>> + cdclk_state = dev_priv->cdclk.hw; >>> + >>> + if (IS_GEMINILAKE(dev_priv)) { >>> + cdclk_state.cdclk = glk_calc_cdclk((2*96000)); >>> + cdclk_state.vco = glk_de_pll_vco(dev_priv, cdclk_state.cdclk); >>> + cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk); >>> + bxt_set_cdclk(dev_priv, &cdclk_state); >>> + } >>> +} >>> + >>> +/** >>> * bxt_uninit_cdclk - Uninitialize CDCLK on BXT >>> * @dev_priv: i915 device >>> * >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>> index d1452fd2a58d..5192753df3dc 100644 >>> --- a/drivers/gpu/drm/i915/intel_drv.h >>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>> @@ -1417,6 +1417,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv); >>> void cnl_init_cdclk(struct drm_i915_private *dev_priv); >>> void cnl_uninit_cdclk(struct drm_i915_private *dev_priv); >>> void bxt_init_cdclk(struct drm_i915_private *dev_priv); >>> +void intel_cdclk_bump(struct drm_i915_private *dev_priv); >>> void bxt_uninit_cdclk(struct drm_i915_private *dev_priv); >>> void icl_init_cdclk(struct drm_i915_private *dev_priv); >>> void icl_uninit_cdclk(struct drm_i915_private *dev_priv); >> -- >> Jani Nikula, Intel Open Source Technology Center >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 09 Apr 2018, "Kumar, Abhay" <abhay.kumar@intel.com> wrote: > Dynamic cdclk is disabled in BIOS/GOP hence gop makes it to highest > clock i.e 316.8. Will attach logs with drm debug enabled in bug. > I am also inclined towards 192 which will make max cdclk.. Please also attach /sys/kernel/debug/dri/0/i915_vbt to the bug. It doesn't look like we look at the VBT dynamic cdclk field. Does dynamic debug disabled mean we should go for max? BR, Jani.
On Tue, 10 Apr 2018, Jani Nikula <jani.nikula@intel.com> wrote: > On Mon, 09 Apr 2018, "Kumar, Abhay" <abhay.kumar@intel.com> wrote: >> Dynamic cdclk is disabled in BIOS/GOP hence gop makes it to highest >> clock i.e 316.8. Will attach logs with drm debug enabled in bug. >> I am also inclined towards 192 which will make max cdclk.. > > Please also attach /sys/kernel/debug/dri/0/i915_vbt to the bug. > > It doesn't look like we look at the VBT dynamic cdclk field. Does > dynamic debug disabled mean we should go for max? Ville, I tried to look at how to disable dynamic cdclk for some platforms based on the VBT, but it gets a bit hairy. The code seems pretty wired for going to lowest possible. I've got the trivial VBT parsing part, but plugging that into the cdclk code in a clean way that could *also* be backported to stable is driving me nuts. Any ideas? I'd like to fix the issue first, and (then have you ;) embark on the refactoring afterwards. It's trivial to plug the check into intel_crtc_compute_min_cdclk() and return dev_priv->max_cdclk_freq, but a) I think that place should be reserved for crtc_state related limitations, and seems the completely wrong place to do system level things, b) it's not optimal to go through all the cdclk code to do nothing in the end, and c) it doesn't work for the no crtc's active case at init time. Just setting the .set_cdclk and .modeset_calc_cdclk hooks to NULL was another idea, but the hooks get initialized before VBT parsing. And I don't think that works for init either. BR, Jani.
On 4/10/2018 1:49 AM, Nikula, Jani wrote: > On Tue, 10 Apr 2018, Jani Nikula <jani.nikula@intel.com> wrote: >> On Mon, 09 Apr 2018, "Kumar, Abhay" <abhay.kumar@intel.com> wrote: >>> Dynamic cdclk is disabled in BIOS/GOP hence gop makes it to highest >>> clock i.e 316.8. Will attach logs with drm debug enabled in bug. >>> I am also inclined towards 192 which will make max cdclk.. >> Please also attach /sys/kernel/debug/dri/0/i915_vbt to the bug. >> >> It doesn't look like we look at the VBT dynamic cdclk field. Does >> dynamic debug disabled mean we should go for max? currently in kernel we don't honor this field. GOP does and by disabling it cdclk will run at max speed. attached vbt dump in bug. with patchset 1 i found one issue where while resuming HDA takes 4 second and also that time cdclk is 79.2 below logs shows which function takes more time. 78.485868] Suspending console(s) (use no_console_suspend to debug) [ 78.521654] HDMI HDA Codec ehdaudio0D2: Enter: hdmi_codec_prepare [ 78.620851] HDMI HDA Codec ehdaudio0D2: Enter1: hdac_hdmi_runtime_resume [ 78.620856] HDMI HDA Codec ehdaudio0D2: Enter2: hdac_hdmi_runtime_resume [ 78.620863] HDMI HDA Codec ehdaudio0D2: Enter3: hdac_hdmi_runtime_resume [ 78.620878] HDMI HDA Codec ehdaudio0D2: Enter4: hdac_hdmi_runtime_resume [ 78.620886] cdclk_1 79200 [ 78.624431] cdclk_1 79200 [ 78.626222] HDMI HDA Codec ehdaudio0D2: Enter5: hdac_hdmi_runtime_resume [ 78.626226] HDMI HDA Codec ehdaudio0D2: Enter6: hdac_hdmi_runtime_resume [ 79.629307] HDMI HDA Codec ehdaudio0D2: Enter7: hdac_hdmi_runtime_resume [ 80.632308] HDMI HDA Codec ehdaudio0D2: Enter8: hdac_hdmi_runtime_resume [ 81.635303] HDMI HDA Codec ehdaudio0D2: Exit: hdac_hdmi_runtime_resume [ 82.638302] HDMI HDA Codec ehdaudio0D2: Exit: hdmi_codec_prepare [ 82.638348] calling input11+ @ 2310, parent: card0 [ 82.638353] call input11+ returned 0 after 1 usecs but when i hardcode cdcdlk glk_calc_cdclk minimum 158.4 then there is no 4sec delay in these function. > Ville, I tried to look at how to disable dynamic cdclk for some > platforms based on the VBT, but it gets a bit hairy. The code seems > pretty wired for going to lowest possible. I've got the trivial VBT > parsing part, but plugging that into the cdclk code in a clean way that > could *also* be backported to stable is driving me nuts. Any ideas? I'd > like to fix the issue first, and (then have you ;) embark on the > refactoring afterwards. > > It's trivial to plug the check into intel_crtc_compute_min_cdclk() and > return dev_priv->max_cdclk_freq, but a) I think that place should be > reserved for crtc_state related limitations, and seems the completely > wrong place to do system level things, b) it's not optimal to go through > all the cdclk code to do nothing in the end, and c) it doesn't work for > the no crtc's active case at init time. > > Just setting the .set_cdclk and .modeset_calc_cdclk hooks to NULL was > another idea, but the hooks get initialized before VBT parsing. And I > don't think that works for init either. > > BR, > Jani. > >
On Tue, Apr 10, 2018 at 08:00:10PM -0700, Kumar, Abhay wrote: > > > On 4/10/2018 1:49 AM, Nikula, Jani wrote: > > On Tue, 10 Apr 2018, Jani Nikula <jani.nikula@intel.com> wrote: > >> On Mon, 09 Apr 2018, "Kumar, Abhay" <abhay.kumar@intel.com> wrote: > >>> Dynamic cdclk is disabled in BIOS/GOP hence gop makes it to highest > >>> clock i.e 316.8. Will attach logs with drm debug enabled in bug. > >>> I am also inclined towards 192 which will make max cdclk.. > >> Please also attach /sys/kernel/debug/dri/0/i915_vbt to the bug. > >> > >> It doesn't look like we look at the VBT dynamic cdclk field. Does > >> dynamic debug disabled mean we should go for max? > currently in kernel we don't honor this field. GOP does and by disabling > it cdclk will run at max speed. I don't particularly like the idea of using that vbt field. I'm not aware of any cases where changing cdclk would be totally busted in the hw, so I don't really see why this thing should even exist in the vbt, apart from some questionable marketing purposes. I think there's plenty of other stuff like that in the vbt which we happily ignore as well. Depending on how common it is to have it set in the vbt it could also drastically reduce our testing coverage of the cdclk code.
On Mon, Apr 09, 2018 at 03:11:38PM -0700, Kumar, Abhay wrote: > > > On 4/9/2018 3:33 AM, Ville Syrjälä wrote: > > On Fri, Apr 06, 2018 at 04:47:08PM +0300, Jani Nikula wrote: > >> On Thu, 05 Apr 2018, Abhay Kumar <abhay.kumar@intel.com> wrote: > >>> In glk when device boots with 1366x768 panel, HDA codec doesn't comeup. > >>> This result in no audio forever as cdclk is < 96Mhz. > >>> This chagne will ensure CD clock to be twice of BCLK. > >> In short, we can't poke around CDCLK like this. It needs a full modeset, > >> and it's non-trivial from the path you're calling this from. > > I tried to cobble something together quickly: > > git://github.com/vsyrjala/linux.git glk_cnl_cdclk_audio > > > > Not tested at all, and I have no idea if my assumptions about > > snd_hda_intel actually hold. > Hi Ville, > > Tried your patch but as soon as it enters "glk_force_audio_cdclk" > the device locks up and reboots. waited for 30-40 times and it always > reboot at same place. > It never reaches Chrome OS login screen. That likely means my assumptions about snd_hda_intel weren't true. So probably we'll need to perform some surgery on snd_hda_intel as well.
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 709d6ca68074..f7dd3d532e93 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -723,15 +723,37 @@ static void i915_audio_component_put_power(struct device *kdev) intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO); } +/* Get CDCLK in kHz */ +static int i915_audio_component_get_cdclk_freq(struct device *kdev) +{ + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); + + if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) + return -ENODEV; + + return dev_priv->cdclk.hw.cdclk; +} + static void i915_audio_component_codec_wake_override(struct device *kdev, bool enable) { struct drm_i915_private *dev_priv = kdev_to_i915(kdev); u32 tmp; + int current_cdclk; if (!IS_GEN9_BC(dev_priv)) return; + current_cdclk = i915_audio_component_get_cdclk_freq(kdev); + + /* + * Before probing for HDA Codec we need to make sure + * "The CD clock frequency must be at least twice + * the frequency of the Azalia BCLK." + */ + if (INTEL_GEN(dev_priv) >= 9 && current_cdclk <= 192000) + intel_cdclk_bump(dev_priv); + i915_audio_component_get_power(kdev); /* @@ -753,17 +775,6 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, i915_audio_component_put_power(kdev); } -/* Get CDCLK in kHz */ -static int i915_audio_component_get_cdclk_freq(struct device *kdev) -{ - struct drm_i915_private *dev_priv = kdev_to_i915(kdev); - - if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) - return -ENODEV; - - return dev_priv->cdclk.hw.cdclk; -} - /* * get the intel_encoder according to the parameter port and pipe * intel_encoder is saved by the index of pipe diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index dc7db8a2caf8..9426e1b7badc 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1516,6 +1516,27 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv) } /** + * intel_cdclk_bump - Increase cdclk to 2* BCLK + * @dev_priv: i915 device + * + * Increase CDCLK for GKL and CNL. This is done only + * during HDA codec probe. + */ +void intel_cdclk_bump(struct drm_i915_private *dev_priv) +{ + struct intel_cdclk_state cdclk_state; + + cdclk_state = dev_priv->cdclk.hw; + + if (IS_GEMINILAKE(dev_priv)) { + cdclk_state.cdclk = glk_calc_cdclk((2*96000)); + cdclk_state.vco = glk_de_pll_vco(dev_priv, cdclk_state.cdclk); + cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk); + bxt_set_cdclk(dev_priv, &cdclk_state); + } +} + +/** * bxt_uninit_cdclk - Uninitialize CDCLK on BXT * @dev_priv: i915 device * diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d1452fd2a58d..5192753df3dc 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1417,6 +1417,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv); void cnl_init_cdclk(struct drm_i915_private *dev_priv); void cnl_uninit_cdclk(struct drm_i915_private *dev_priv); void bxt_init_cdclk(struct drm_i915_private *dev_priv); +void intel_cdclk_bump(struct drm_i915_private *dev_priv); void bxt_uninit_cdclk(struct drm_i915_private *dev_priv); void icl_init_cdclk(struct drm_i915_private *dev_priv); void icl_uninit_cdclk(struct drm_i915_private *dev_priv);
In glk when device boots with 1366x768 panel, HDA codec doesn't comeup. This result in no audio forever as cdclk is < 96Mhz. This chagne will ensure CD clock to be twice of BCLK. v2: - Address comment (Jani) - New design approach Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937 Signed-off-by: Abhay Kumar <abhay.kumar@intel.com> --- drivers/gpu/drm/i915/intel_audio.c | 33 ++++++++++++++++++++++----------- drivers/gpu/drm/i915/intel_cdclk.c | 21 +++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 44 insertions(+), 11 deletions(-)