| Message ID | 20191231144718.32127-1-kai.vehmanen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: save state of AUD_FREQ_CNTRL on all gen9+ platforms | expand |
On Tue, Dec 31, 2019 at 04:47:18PM +0200, Kai Vehmanen wrote: > On old platforms the default values of AUD_FREQ_CNTRL are > typically used (as set by BIOS), so this has not been an issue, > but future platforms will definitely need this. Extend the state > save logic to cover all gen9+ platforms. > > Bspec: 49281 > Cc: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Looks good to me. Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Given that the lack of this save/restore was causing noticeable problems on ICL/TGL, do you know whether the same problems were also seen on EHL/JSL? If so, we may want Cc: stable and Fixes: tags so that it gets backported? Matt > --- > drivers/gpu/drm/i915/display/intel_audio.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c > index 27710098d056..e6517c5d83ae 100644 > --- a/drivers/gpu/drm/i915/display/intel_audio.c > +++ b/drivers/gpu/drm/i915/display/intel_audio.c > @@ -849,7 +849,7 @@ static unsigned long i915_audio_component_get_power(struct device *kdev) > ret = intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO); > > if (dev_priv->audio_power_refcount++ == 0) { > - if (IS_TIGERLAKE(dev_priv) || IS_ICELAKE(dev_priv)) { > + if (INTEL_GEN(dev_priv) >= 9) { > I915_WRITE(AUD_FREQ_CNTRL, dev_priv->audio_freq_cntrl); > DRM_DEBUG_KMS("restored AUD_FREQ_CNTRL to 0x%x\n", > dev_priv->audio_freq_cntrl); > @@ -1124,7 +1124,7 @@ static void i915_audio_component_init(struct drm_i915_private *dev_priv) > return; > } > > - if (IS_TIGERLAKE(dev_priv) || IS_ICELAKE(dev_priv)) { > + if (INTEL_GEN(dev_priv) >= 9) { > dev_priv->audio_freq_cntrl = I915_READ(AUD_FREQ_CNTRL); > DRM_DEBUG_KMS("init value of AUD_FREQ_CNTRL of 0x%x\n", > dev_priv->audio_freq_cntrl); > -- > 2.17.1 >
Hey, On Tue, 31 Dec 2019, Matt Roper wrote: > On Tue, Dec 31, 2019 at 04:47:18PM +0200, Kai Vehmanen wrote: > > On old platforms the default values of AUD_FREQ_CNTRL are > > typically used (as set by BIOS), so this has not been an issue, > > but future platforms will definitely need this. Extend the state > > save logic to cover all gen9+ platforms. [...] > Given that the lack of this save/restore was causing noticeable problems > on ICL/TGL, do you know whether the same problems were also seen on > EHL/JSL? If so, we may want Cc: stable and Fixes: tags so that it gets > backported? the fix is most critical for TGL and later (due to changed hw default values gen12 display onwards). For EHL/JSL, this would seem less important as systems are shipping using the hw default configuration in which case this patch is not needed. Based on current data, I'd probably skip the Cc stable at this point as TGL is already covered -- or limit to "v5.5+". PS A newbie question, if decision is to cc stable, should I add it as the original submitted and resend V2, or are stable tags typically added by the intel-gfx maintainers when applying a patch (i.e. no actions needed from me). In sound tree, latter seems to be the norm.. I see both conventions used here on intel-gfx. Br, Kai
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index 27710098d056..e6517c5d83ae 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -849,7 +849,7 @@ static unsigned long i915_audio_component_get_power(struct device *kdev) ret = intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO); if (dev_priv->audio_power_refcount++ == 0) { - if (IS_TIGERLAKE(dev_priv) || IS_ICELAKE(dev_priv)) { + if (INTEL_GEN(dev_priv) >= 9) { I915_WRITE(AUD_FREQ_CNTRL, dev_priv->audio_freq_cntrl); DRM_DEBUG_KMS("restored AUD_FREQ_CNTRL to 0x%x\n", dev_priv->audio_freq_cntrl); @@ -1124,7 +1124,7 @@ static void i915_audio_component_init(struct drm_i915_private *dev_priv) return; } - if (IS_TIGERLAKE(dev_priv) || IS_ICELAKE(dev_priv)) { + if (INTEL_GEN(dev_priv) >= 9) { dev_priv->audio_freq_cntrl = I915_READ(AUD_FREQ_CNTRL); DRM_DEBUG_KMS("init value of AUD_FREQ_CNTRL of 0x%x\n", dev_priv->audio_freq_cntrl);
On old platforms the default values of AUD_FREQ_CNTRL are typically used (as set by BIOS), so this has not been an issue, but future platforms will definitely need this. Extend the state save logic to cover all gen9+ platforms. Bspec: 49281 Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> --- drivers/gpu/drm/i915/display/intel_audio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)