On Thu, Oct 20, 2011 at 12:35:40PM -0400, Devin Heitmueller wrote: > On Thu, Oct 20, 2011 at 12:23 PM, Janne Grunau <janne@xxxxxxxxxx> wrote: > > > > I've looked at them only at very beginning and if I recall correctly > > they had no visible effects. The values in the linux driver were taken > > from sniffing the windows driver. I remember that I've verified the > > default brightness value since 0x86 looked odd. I'm not sure that I > > verified all controls. I might have assumed all controls shared the > > same value range. > > > > There were previous reports of the picture controls not working at all. > > Thanks for taking the time to chime in. no problem, sorry for ignoring the other mails, I had no time to look at the problem immediately and then forgot about it. > If the controls really were broken all along under Linux, then that's > good to know. That said, I'm not confident the changes Taylor > proposed should really be run against older firmwares. There probably > needs to be a check to have the values in question only applied if > firmware >= 16. If the controls were broken entirely, then we should > probably not advertise them in ENUM_CTRL and S_CTRL should return > -EINVAL if running the old firmware (perhaps put a warning in the > dmesg output saying the controls are unavailable because the user is > not running firmware >= 16). > > My immediate concern is about ensuring we don't cause breakage in > older firmware. For example, we don't know if there are some older > firmware revisions that *did* work with the driver. The controls > might have worked up to firmware revision 10, then been broken from > 11-15, then work again in 16 (with the new hue value needed). The > safe approach is to only use these new settings if they're running > firmware >= 16. I think such scenario is unlikely but I don't know it for sure and I don't want to force anyone to test every firmware version. Ignoring them for firmware version < 16 should be safe since we assume they had no effect. Returning -EINVAL might break API-ignoring applications written with the HD PVR in mind but I think it's a better approach than silently ignoring those controls. Janne -- 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