Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"

["Lofstedt, Marta" <marta.lofstedt@intel.com>]

See bug: https://bugs.freedesktop.org/show_bug.cgi?id=105069

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Saarinen, Jani
> Sent: Tuesday, February 13, 2018 9:35 AM
> To: Kumar, Abhijeet <abhijeet.kumar@intel.com>; Takashi Iwai
> <tiwai@suse.de>; Chris Wilson <chris@chris-wilson.co.uk>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] AWOOGA: Revert "ALSA: hda: Make use of
> core codec functions to sync power state"
> 
> Hi,
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> > Behalf Of Kumar, Abhijeet
> > Sent: tiistai 13. helmikuuta 2018 7.18
> > To: Takashi Iwai <tiwai@suse.de>; Chris Wilson
> > <chris@chris-wilson.co.uk>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] AWOOGA: Revert "ALSA: hda: Make use
> > of core codec functions to sync power state"
> >
> >
> >
> > On 2/13/2018 10:41 AM, Takashi Iwai wrote:
> > > On Mon, 12 Feb 2018 18:29:53 +0100,
> > > Chris Wilson wrote:
> > >> This reverts commit 3b5b899ca67db07a4c4825911072221f99e157e2.
> > >>
> > >> Fixes: 3b5b899ca67d ("ALSA: hda: Make use of core codec functions
> > >> to sync power state")
> > >> Cc: Abhijeet Kumar <abhijeet.kumar@intel.com>
> > >> Cc: Takashi Iwai <tiwai@suse.de>
> > > Did the patch break anything?
> > Not sure, why it would break anything. As there was literally no
> > functional change.
> You can also doing so by yourself just looking at resutls from revert, but I also
> sent mail to explain.
> 
> >
> > Chris,
> > Can you help us in understanding the issue caused by this patch ?
> >
> > Warm Regards,
> > Abhijeet
> > > I don't understand it without any real context...
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > >> ---
> > >>   sound/pci/hda/hda_codec.c | 28 +++++++++++++++++++++++++++-
> > >>   sound/pci/hda/hda_local.h |  6 +-----
> > >>   2 files changed, 28 insertions(+), 6 deletions(-)
> > >>
> > >> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > >> index 5bc3a7468e17..e018ecbf78a8 100644
> > >> --- a/sound/pci/hda/hda_codec.c
> > >> +++ b/sound/pci/hda/hda_codec.c
> > >> @@ -2702,6 +2702,32 @@ void
> snd_hda_codec_set_power_to_all(struct
> > hda_codec *codec, hda_nid_t fg,
> > >>   }
> > >>   EXPORT_SYMBOL_GPL(snd_hda_codec_set_power_to_all);
> > >>
> > >> +/*
> > >> + * wait until the state is reached, returns the current state  */
> > >> +static unsigned int hda_sync_power_state(struct hda_codec *codec,
> > >> +
> hda_nid_t fg,
> > >> +
> unsigned int power_state)
> > >> +{
> > >> +	unsigned long end_time = jiffies + msecs_to_jiffies(500);
> > >> +	unsigned int state, actual_state;
> > >> +
> > >> +	for (;;) {
> > >> +		state = snd_hda_codec_read(codec, fg, 0,
> > >> +
> AC_VERB_GET_POWER_STATE, 0);
> > >> +		if (state & AC_PWRST_ERROR)
> > >> +			break;
> > >> +		actual_state = (state >> 4) & 0x0f;
> > >> +		if (actual_state == power_state)
> > >> +			break;
> > >> +		if (time_after_eq(jiffies, end_time))
> > >> +			break;
> > >> +		/* wait until the codec reachs to the target state
> */
> > >> +		msleep(1);
> > >> +	}
> > >> +	return state;
> > >> +}
> > >> +
> > >>   /**
> > >>    * snd_hda_codec_eapd_power_filter - A power filter callback for
> EAPD
> > >>    * @codec: the HDA codec
> > >> @@ -2764,7 +2790,7 @@ static unsigned int
> > >> hda_set_power_state(struct
> > hda_codec *codec,
> > >>
> 	   state);
> > >>
> 	snd_hda_codec_set_power_to_all(codec, fg,
> > power_state);
> > >>   		}
> > >> -		state = snd_hda_sync_power_state(codec, fg,
> power_state);
> > >> +		state = hda_sync_power_state(codec, fg,
> power_state);
> > >>   		if (!(state & AC_PWRST_ERROR))
> > >>   			break;
> > >>   	}
> > >> diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> > >> index 321e78baa63c..5b5c324c99b9 100644
> > >> --- a/sound/pci/hda/hda_local.h
> > >> +++ b/sound/pci/hda/hda_local.h
> > >> @@ -622,11 +622,7 @@ snd_hda_check_power_state(struct
> hda_codec
> > *codec, hda_nid_t nid,
> > >>   {
> > >>   	return snd_hdac_check_power_state(&codec->core, nid,
> target_state);
> > >>   }
> > >> -static inline bool snd_hda_sync_power_state(struct hda_codec
> *codec,
> > >> -			   hda_nid_t nid, unsigned int
> target_state)
> > >> -{
> > >> -	return snd_hdac_sync_power_state(&codec->core, nid,
> target_state);
> > >> -}
> > >> +
> > >>   unsigned int snd_hda_codec_eapd_power_filter(struct hda_codec
> *codec,
> > >>
> hda_nid_t nid,
> > >>
> unsigned int power_state);
> > >> --
> > >> 2.16.1
> > >>
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx