maemo.org - Talk

maemo.org - Talk (https://talk.maemo.org/index.php)
-   Maemo 5 / Fremantle (https://talk.maemo.org/forumdisplay.php?f=40)
-   -   [Development] kernel-power (https://talk.maemo.org/showthread.php?t=78371)

vi_ 2012-02-01 09:07

Re: [Development] kernel-power
 
Wait, some kind of highpass filter? WTF for? Can we have some details on this?

Is it this?

From this guy?

who said this:

Quote:

Originally Posted by luke-jr
cuz I have nfc how any of that works or what a highpass filter even is

Quote:

Originally Posted by luke-jr
apparently it only works at 48 kHz as-is tho I wouldn't have a clue how to adapt for other sample rates

Quote:

Originally Posted by luke-jr
that assumes I have some idea what f is, or really which part of this code does the filtering at all :P

Quote:

Originally Posted by luke-jr
maybe I'll find a way to disable other sample rates in the kernel.

I know I am not in much of a position to criticise, being much of a non-contributer however this one has my spidey senses tingling.

freemangordon 2012-02-01 09:40

Re: [Development] kernel-power
 
Well, I think I am in position :p

@Pali, why is that filter needed in Maemo5 kernel, what problem/bug it fixes, or what functionality it adds? Who sent you the patch? Is it upstream patch?

TBH I think you should remove that from kernel-power at least until there is some information WTF is this animal(the filter) doing and who will support it (fix bugs) at least while KP is in devel.

Or even better - remove it constantly.

P.S.
And maybe you should re-consider your policy to accept patches from developers who cannot be contacted/don't want to help if some problem arise, thus leaving only me and you to fix bugs introduced by some random guy.

don_falcone 2012-02-01 10:00

Re: [Development] kernel-power
 
Quote:

+The codec driver leverages the codecs effects through alsa controls and a
+hwdep device for controlling the hardware fourth-order IIR filter block.
+
+There's an alsa control, "3D Control - Depth" for depth simulation.
+The rest of the controls are for the IIR filter:
+
+1- A control for setting the bass/treble gain, which sets the filter's
+ coefficients to certain precalculated values.
+2- A control for 'off' / 'Bass/Treble' / 'Custom'. 'Bass/Treble' means
+ the bass/treble gain controls are used, while 'custom' means the
+ coefficients have been set through the hwdep device (see below).
+Note: bass/treble controls are not yet implemented
+
+Filters
+--------
+Note: Setting a filter's coeffs automatically turns it off, it needs to
+be turned on explicitly.
+
+The De-emphasis filter can only be controlled on the machine driver level.
+For example for the n900 (rx51.c) it is used as highpass filter for
+speaker protection. See tlv320aic3x.h, aic3x_deemph_set_* for details.
Quote:

/* Default De-emphasis filter coefficients to use as a highpass for
+ * cheap speaker protection */
Alles klar. We will get weak sound from the speakers.... I wonder if this applies to headphone/TV out too.

Estel 2012-02-01 10:10

Re: [Development] kernel-power
 
Quote:

Originally Posted by pali (Post 1158367)
Now kernel-power has 4 major packages (needed for using):
kernel-power
kernel-power-flasher
kernel-power-modules
kernel-power-bootimg
and some more for development...

I have question: Why two packages are needed for flashing kernel? kernel-power only contains fiasco image (with packaged zImage) and kernel-power-flasher has only postinst script which flash that fiasco image (from package kernel-power) and then REMOVE fiasco image from rootfs. (Note that stock kernel has same process, kernel-power only copied kernel source package)

What do you think about merging kernel-power and kernel-power-flasher to one package kernel-power-flasher? (I will do not delete kernel-power package, I only mark it as dummy/transitional)

Positive on this change:
* you do not need to install kernel-power package
* when you want to reinstall/reflash kernel-power, you need only reinstall package kernel-power-flasher

Negative:
* packages which depends directly on kernel-power will must change dependences to kernel-power-flasher

But I will let kernel-power package in repository, but it will be empty (+ added dependency on kernel-power-flasher) - so this does not break last negative point.

So what do you think?

I think it's great idea. Yet, are You sure, that both kernel-power-bootimg and kernel-power-flasher are needed for usage? For people with multiboot, kernel-power-bootimg is enough (I haven't installed kernel-power-flasher, and everything works fine).

/Estel

// Edit

+1 on kicking highpass-filter "patch" from KP, and not accepting patches from random noobs, at least, without audit.

don_falcone 2012-02-01 10:16

Re: [Development] kernel-power
 
From a modularized & "clean & neat & tidied-up" POV, it makes sense to me having separate packages for actual kernel image and the flasher. It's almost comparable in splitting firmware image(s) and driver (loader/flasher).

EDIT: Maybe it's debatable why the actual image is removed after flashing, i see some congruency with kernel-power-bootimg here, having two actual kernel images (IIRC, they are same format?)

misterc 2012-02-01 10:36

Re: [Development] kernel-power
 
Quote:

Originally Posted by pali (Post 1158367)
Now kernel-power has 4 major packages (needed for using):
kernel-power
kernel-power-flasher
kernel-power-modules
kernel-power-bootimg
and some more for development...

[...]
So what do you think?

i'm not much of a (kernel / OS) dev, so i'm just talking from a (SW) admin point of view, but i second Don_falcone's opinion; keep things clearly structured and leave it up to the end user to leave out what they don't want (as Estel did).

also, more from a "distribution" and SW management point of view, is it smart to change the structure of the KP (meta-)package?
i know the KP is not a "normal" package as it can not be uninstalled (or even upgraded?) thru HAM/FAM.

maybe an idea (changing strucutre) if you decide to include the KP in CSSU?

pali 2012-02-01 11:21

Re: [Development] kernel-power
 
Patch Support-for-tlv320aic3x-codec-highpass-filter-needed.diff was disabled.

I sent info to luke-jr via jabber, to comment this patch.

===

Also in future for better patch review from other community members:
Each new patch must be linked to THIS thread and commented/reviewed/tested by more (at least 3) people.

I think this should be enought.

Luke-Jr 2012-02-01 14:45

Re: [Development] kernel-power
 
So is there actually a real problem with Support-for-tlv320aic3x-codec-highpass-filter-needed.diff? If you guys would prefer to potentially blow up your speakers, so be it...

vi_ 2012-02-01 14:56

Re: [Development] kernel-power
 
Quote:

Originally Posted by Luke-Jr (Post 1158966)
So is there actually a real problem with Support-for-tlv320aic3x-codec-highpass-filter-needed.diff? If you guys would prefer to potentially blow up your speakers, so be it...

No offence intended, it is just that none of us know what this is or what it does and the IRC conversation highlighted above does not fill me with confidence.

Would you care to explain what this patch does, how it achieves it and how the end user can control it's effect?

Are you 'mnzaki'?

Luke-Jr 2012-02-01 14:57

Re: [Development] kernel-power
 
No idea, I didn't write it. Read the documentation included.


All times are GMT. The time now is 06:03.

vBulletin® Version 3.8.8