Re: [PATCH 3/7] ALSA: hda/intel: Drop superfluous AZX_DCAPS_I915_POWERWELL checks

[Takashi Iwai <tiwai@suse.de>]

On Mon, 10 Dec 2018 21:56:15 +0100,
Pierre-Louis Bossart wrote:
> 
> 
> On 12/9/18 3:33 AM, Takashi Iwai wrote:
> > Since snd_hdac_display_power() can be called even for a HDA controller
> > without DRM binding, lots of superfluous AZX_DCAPS_I915_POWERWELL
> > checks in hda_intel.c can be dropped.  This simplifies the code a
> > lot.
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >   sound/pci/hda/hda_intel.c | 43 +++++++++++++++------------------------
> >   1 file changed, 16 insertions(+), 27 deletions(-)
> >
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index 151c6ca85ec6..cacee33a74a8 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -948,9 +948,7 @@ static void __azx_runtime_suspend(struct azx *chip)
> >   	azx_stop_chip(chip);
> >   	azx_enter_link_reset(chip);
> >   	azx_clear_irq_pending(chip);
> > -	if ((chip->driver_caps & AZX_DCAPS_I915_POWERWELL) &&
> > -	    hda->need_i915_power)
> > -		display_power(chip, false);
> > +	display_power(chip, false);
> >   }
> >     static void __azx_runtime_resume(struct azx *chip)
> > @@ -960,11 +958,9 @@ static void __azx_runtime_resume(struct azx *chip)
> >   	struct hda_codec *codec;
> >   	int status;
> >   -	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
> > -		display_power(chip, true);
> > -		if (hda->need_i915_power)
> > -			snd_hdac_i915_set_bclk(bus);
> > -	}
> > +	display_power(chip, true);
> > +	if (hda->need_i915_power)
> > +		snd_hdac_i915_set_bclk(bus);
> 
> Question: I still see this 'old style' init in hda_intel.c even with
> all the patches applied.
> 
>     /* initialize chip */
>     azx_init_pci(chip);
> 
>     if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
>         snd_hdac_i915_set_bclk(bus);
> 
> is this intentional or a miss?

It's intentional.  The purpose of the patch isn't to eliminate the
whole DCAPS_I915_POWERWELL checks, but remove the checks that are
relevant with snd_hdac_display_power() calls.  In the changes, some
calls are reduced with only hda->need_i915 check, which is safe since
need_i915 mandates AZX_DCAPS_I915_POWERWELL.

Though, actually, snd_hdac_i915_set_bclk() can be called safely for
the case without GPU binding, too.  So it's fine to get rid of the
AZX_DCAPS check there, too.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel