[alsa-devel] [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.

[Li Bo]

On Tue, Oct 6, 2009 at 11:34 PM, Takashi Iwai <tiwai at suse.de> wrote:
> At Tue, 6 Oct 2009 23:27:35 +0800,
> Li Bo wrote:
>>
>> On Tue, Oct 6, 2009 at 8:38 PM, Takashi Iwai <tiwai at suse.de> wrote:
>> > At Tue, 6 Oct 2009 14:17:40 +0200,
>> > Harald Welte wrote:
>> >>
>> >> Hi all,
>> >>
>> >> On Tue, Oct 06, 2009 at 08:01:40AM +0200, Takashi Iwai wrote:
>> >>
>> >> > > As you describe, we implement it with self-reschedule workqueue, acting
>> >> > > like a timer.
>> >> > > Is there some advice to implement timer-like task, I tested  with timer and
>> >> > > it will crash the driver.
>> >> >
>> >> > Right, you can't call it in a softirq.
>> >> >
>> >> > A few problems, however:
>> >> > - present variable should be in struct via_spec, instead of a static
>> >> >   variable in update_hp_jack_state().
>> >>
>> >> I agree
>> >> > - This mechanism is unconditionally invoked on VT1708 although, in
>> >> >   theory, you can have a hardware that doesn't need HP jack detection
>> >>
>> >> Yes, Logan/Li/Lydia: If we generate this kind of workaround with a timer,
>> >> it should only be used on those particular chips that absolutely need it.
>> >
>> > As far as I see, the code is only for VT1708, so it's already specific
>> > to the codec chip.  But, even with this chip, you might have a pin
>> > configuration that doesn't need the HP jack detection.
>> >
>>
>> Yes, it's only for VT1708, and HP exists on most cases, and so HP jack detection
>> are needed.
>> And present will put into via_spec.
>
> If it's restricted only during the opened (better running) PCM stream,
> it's not too bad.  But, it'd be even better to check the availability
> of HP jack.
>
Sorry about analog loopback is not considered, I think analog loopback
mute/unmute
callback should also be added, is that OK?

>> >> > - Waking up each 100ms for the *whole* time is really bad regarding
>> >> >   the power-saving; can't it be optimized?
>> >>
>> >> I would personally actually suggest to simply not support headphone plug events
>> >> in case you are using such a [buggy?] chipset that does not support them.
>> >>
>> >> I mean, what do you gain from that event? It's not something super-important.
>> >>
>> >> waking up unconditionally every 100ms is really a too high burden on the
>> >> battery life.
>> >>
>> >> Another option might be to make it a module load time parameter, so people
>> >> can choose if they actually want to pay the high power/batyery price for this
>> >> small feature.
>> >
>> > Yes.
>> >
>> > Basically, if we can drop the analog-loopback feature, the HP jack
>> > probing can be simplified very much.  The probing is needed only when
>> > the PCM is opened/prepared and running.  It can be implemented in a
>> > similar way for the volume-update procedure at PCM trigger.
>> > And, during PCM running, (relatively) high-frequent polling isn't so
>> > critical.
>> >
>> >
>> > thanks,
>> >
>> > Takashi
>> >
>>
>> I know that jack detect on VT1708 that do not support unsolicited
>> response is weird, but
>>  it's for customer request (and so are some other patches:), we just
>> have to do something.
>>
>> I think it's good in open/close callback, I'll update.
>>
>> About self-reschedule workqueue, maybe there are some other choices,
>> or should we keep
>> using it?
>
> A delayed workqueue is OK.  But, it should check some flag before
> rescheduling itself.  Also, don't forget about suspend/resume.
>
>
> thanks,
>
> Takashi
>
Thank you, I will rewrite like this.