On Wed, 16 Feb 2022 09:38:44 +0100,
Amadeusz SX2awiX4ski wrote:
>
> On 2/15/2022 3:01 PM, Kai Vehmanen wrote:
>
> > static void silent_stream_disable(struct hda_codec *codec,
> > @@ -1726,7 +1786,16 @@ static void silent_stream_disable(struct hda_codec *codec,
> > {
> > struct hdmi_spec *spec = codec->spec;
> > struct hdmi_spec_per_cvt *per_cvt;
> > - int cvt_idx;
> > + int cvt_idx, err;
> > +
> > + err = snd_hda_power_up_pm(codec);
> > + if (err < 0 && err != -EACCES) {
> > + codec_err(codec,
> > + "Failed to power up codec for silent stream disable ret=[%d]\n",
> > + err);
> > + snd_hda_power_down_pm(codec);
>
> If power up failed, do you need to power down?
Yes, it's essentially pm_runtime_get(), and you need to put down also
at an error path.
But, what I don't follow in the patch is about the part:
> static void silent_stream_enable(struct hda_codec *codec,
> struct hdmi_spec_per_pin *per_pin)
> {
(snip)
> unlock_out:
> mutex_unlock(&per_pin->lock);
> +
> + if (err || !keep_power)
> + snd_hda_power_down_pm(codec);
So this may leave the power up. But where is the corresponding part
that turns it down? The newly added snd_hda_power_up_pm() in
silent_stream_disable() is paired with the one at the tail of that
function, so it looks like a refcount unbalance. Kai?
thanks,
Takashi
