Em 06-08-2010 12:28, Hans Verkuil escreveu:
> On Thursday 05 August 2010 14:11:09 Andy Walls wrote:
>>>> 3. This caught my eye:
>>>>
>>>> 8 + if (is_cx2388x(state)) {
>>>> 9 + /* for cx23885 volume doesn't work,
>>>> 10 + * the calculation always results in
>>>> 11 + * e4 regardless.
>>>> 12 + */
>>>> 13 + cx25840_write(client, 0x8d4, volume);
>>>> 14 + } else
>>>> 15 + cx25840_write(client, 0x8d4, 228 - (vol * 2));
>>>>
>>>> Why is the result always 0xe4? Is that what the register always reads
>>>> back?
>>>
>>> Yes.
>>
>> The cx25840 module expects the volume control to have a range from
>> 0-65535. It divides the input control value by 512 (i.e. volume >> 9).
>> If you are using a control in the cx23885 driver with a range from 0-100
>> or 0-255, that would explain why it doesn't work.
>>
>> BTW, the actual relationship the cx25840 module is implementing is
>> explained in the end of this old post:
>>
>> http://www.gossamer-threads.com/lists/ivtv/devel/39868#39868
>>
>>
>>
>>>>
>>>> Note that your change won't have an intuitive effect, because you
>>>> dropped the '-' sign in front of the volume. The user's slider control
>>>> will likely work in reverse: up is soft, down is loud.
>>>
>>> This is a mistake on my end, although it represents no known regression given
>>> that 0xe4 note and the lact of audio support in the kernel prior to this
>>> patchset for the cx23885.
>>
>> I guess my real gripe is with the conditional code branch when it is not
>> needed AFAICT, if one feeds the cx25840 module with a volume control
>> range of 0-65535 that it is expecting.
>>
>> IIRC the change set also does not have a complementary conditional code
>> branch when reading out the volume value from the register. (But I
>> don't have time to double check that right now - late for work.)
>>
>> Regards,
>> Andy
>>
>>>> I can say the V4L2 control to cx25840 volume scale mapping is a little
>>>> funny. Here is documentation on the mapping from V4L2 volume control
>>>> values to dB to cx25840 register values:
>>>>
>>>> http://linuxtv.org/hg/v4l-dvb/file/9652f85e688a/linux/drivers/media/video/cx18/cx18-alsa-mixer.c#l38
>>>>
>>>
>>> Thanks.
>>>
>>>> It is non-linear at the bottom end with a reg value of 228 = 0xe4 for
>>>> 3/16ths of the entire range. This is due to the cx25840 module's desire
>>>> to keep volume levels at the same perceptible level between the MSP400
>>>> and the CX2584x chips for users with multiple ivtv cards in their
>>>> machine. (Yet another ivtv legacy.)
>>>>
>>>> You ought to look at what values are coming from the V4L2 volume control
>>>> slider and see what's wrong. A V4L2 volume control value in the range
>>>> 0xee00-0xeeff should correspond to 0 dB gain.
>>>
>>> It's been a while but form memory I added debug to the function and passed the
>>> volume control and found the register values (and calculations to be way off),
>>> certainly not what I expected. I figured this was an issue outside of the realm
>>> of the cx23885 and thus isolated my change to the cx23885 only, not effecting
>>> any other products.
>
> I had a discussion with Mauro on irc about this. The whole 0-65535 range for the
> volume in cx25840 is purely historical: the audio V4L1 'controls' always had that
> fixed range (you couldn't specify min and max values in V4L1) and that was just
> blindly propagated to V4L2.
>
> For V4L2 it would be much cleaner to base the min/max values on the hardware.
> The only disadvantage is that apps that store the default control volume in some
> config file will suddenly max out the volume if that driver changes the min/max
> values for the volume to 0-127 (same for bass, treble, etc).
>
> Effectively you are changing the public API for these devices. Is it worthwhile
> doing this? The engineer in me says 'yes', but from a user-perspective it may
> not be such a great idea after all.
Well, the API has not changed. It won't require any change at the userspace programs.
The only action from users is to store a new default.
While the better would be to not needing to do this, e. g. doing it right since the
first version of the driver, it is OK to change the range, to better reflect the hardware
needs.
Btw, we've done things like that before, like changing the default values for the
color control, at cx88 (since the max value there means 200% of color, instead of
100%).
We just need to be sure that no driver is expecting the old behavior when doing that
change.
>
> I hope this clarifies some of the history behind these ranges.
>
> Regards,
>
> Hans
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
