From patchwork Fri Oct 21 03:33:23 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Taylor Ralph X-Patchwork-Id: 8183 Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from ) id 1RH5rR-0003Ji-Jo; Fri, 21 Oct 2011 05:34:00 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-2) with esmtp id 1RH5rQ-0005fc-GO; Fri, 21 Oct 2011 05:33:32 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752896Ab1JUDd0 (ORCPT + 3 others); Thu, 20 Oct 2011 23:33:26 -0400 Received: from mail-ey0-f174.google.com ([209.85.215.174]:51453 "EHLO mail-ey0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752701Ab1JUDdZ (ORCPT ); Thu, 20 Oct 2011 23:33:25 -0400 Received: by eye27 with SMTP id 27so3467822eye.19 for ; Thu, 20 Oct 2011 20:33:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=z4JUgv8/Clz31bK8qDvu+mMSU/8pxc+oqm9u3v3MsMM=; b=Ih+NJYsNaqI0SVqH+qW5rR+aPDfJ2wGXXxkM3IKmEi5VAgSnyXW+wRVUGjIwbPtLCs BL6adfHESh5cdzT1Sg1NulbledchG6dXOLsq7brz8h7SlO3Mq7I5kU8/CrGi7UvuFsto uT/9A97Xz5WT4thx/0wlo6k4l/90buC+f2ANc= MIME-Version: 1.0 Received: by 10.223.60.73 with SMTP id o9mr22190870fah.18.1319168004042; Thu, 20 Oct 2011 20:33:24 -0700 (PDT) Received: by 10.223.98.208 with HTTP; Thu, 20 Oct 2011 20:33:23 -0700 (PDT) In-Reply-To: References: <20111020162340.GC7530@jannau.net> <20111020170811.GD7530@jannau.net> Date: Thu, 20 Oct 2011 23:33:23 -0400 Message-ID: Subject: Re: [PATCH] [media] hdpvr: update picture controls to support firmware versions > 0.15 From: Taylor Ralph To: Devin Heitmueller Cc: Janne Grunau , linux-media@vger.kernel.org Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2011.10.21.32714 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' FORGED_FROM_GMAIL 0.1, MULTIPLE_RCPTS 0.1, MIME_TEXT_ONLY_MP_MIXED 0.05, BODYTEXTP_SIZE_3000_LESS 0, CTYPE_MULTIPART_NO_QUOTE 0, DATE_TZ_NA 0, WEBMAIL_SOURCE 0, __ANY_URI 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CP_POSSIBLE_EXPLOIT_SUBJ 0, __CP_URI_IN_BODY 0, __CT 0, __CTYPE_HAS_BOUNDARY 0, __CTYPE_MULTIPART 0, __CTYPE_MULTIPART_MIXED 0, __FRAUD_BODY_WEBMAIL 0, __FRAUD_WEBMAIL 0, __FRAUD_WEBMAIL_FROM 0, __FROM_GMAIL 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __PHISH_SPEAR_HTTP_RECEIVED 0, __PHISH_SPEAR_STRUCTURE_1 0, __SANE_MSGID 0, __TO_MALFORMED_2 0, __URI_NO_WWW 0, __URI_NS ' X-LSpam-Score: 1.1 (+) X-LSpam-Report: No, score=1.1 required=5.0 tests=BAYES_00=-1.9, FREEMAIL_FROM=0.001, KB_DATE_CONTAINS_TAB=2.751, RCVD_IN_DNSWL_MED=-2.3, TAB_IN_FROM=2.494, T_DKIM_INVALID=0.01, T_TVD_MIME_EPI=0.01 autolearn=no On Thu, Oct 20, 2011 at 3:26 PM, Taylor Ralph wrote: > On Thu, Oct 20, 2011 at 2:14 PM, Devin Heitmueller > wrote: >> On Thu, Oct 20, 2011 at 1:08 PM, Janne Grunau wrote: >>> 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. >> >> At this point, let's just make it so that the old behavior is >> unchanged for old firmwares, meaning from both an API standpoint as >> well as what the values are.  At some point if somebody cares enough >> to go back and fix the support so that the controls actually work with >> old firmwares, they can take that up as a separate task.  In reality, >> it is likely that nobody will ever do that, as the "easy answer" is >> just to upgrade to firmware 16. >> >> Taylor, could you please tweak your patch to that effect and resubmit? >> > > Sure, I'll try to get to it tonight and have it tested. > OK, I've updated the patch per your requests. I made this patch against the latest kernel source but I'm unable to test since my 2.6.32 kernel has symbol issues with the new v4l code. Regards. Acked-by: Jarod Wilson Reviewed-by: Jarod Wilson --- Taylor diff --git a/drivers/media/video/hdpvr/hdpvr-core.c b/drivers/media/video/hdpvr/hdpvr-core.c index 441dacf..687282d 100644 --- a/drivers/media/video/hdpvr/hdpvr-core.c +++ b/drivers/media/video/hdpvr/hdpvr-core.c @@ -154,10 +154,20 @@ static int device_authorization(struct hdpvr_device *dev) } #endif + dev->fw_ver = dev->usbc_buf[1]; + v4l2_info(&dev->v4l2_dev, "firmware version 0x%x dated %s\n", - dev->usbc_buf[1], &dev->usbc_buf[2]); + dev->fw_ver, &dev->usbc_buf[2]); + + if (dev->fw_ver > 0x15) { + dev->options.brightness = 0x80; + dev->options.contrast = 0x40; + dev->options.hue = 0xf; + dev->options.saturation = 0x40; + dev->options.sharpness = 0x80; + } - switch (dev->usbc_buf[1]) { + switch (dev->fw_ver) { case HDPVR_FIRMWARE_VERSION: dev->flags &= ~HDPVR_FLAG_AC3_CAP; break; @@ -169,7 +179,7 @@ static int device_authorization(struct hdpvr_device *dev) default: v4l2_info(&dev->v4l2_dev, "untested firmware, the driver might" " not work.\n"); - if (dev->usbc_buf[1] >= HDPVR_FIRMWARE_VERSION_AC3) + if (dev->fw_ver >= HDPVR_FIRMWARE_VERSION_AC3) dev->flags |= HDPVR_FLAG_AC3_CAP; else dev->flags &= ~HDPVR_FLAG_AC3_CAP; @@ -270,6 +280,8 @@ static const struct hdpvr_options hdpvr_default_options = { .bitrate_mode = HDPVR_CONSTANT, .gop_mode = HDPVR_SIMPLE_IDR_GOP, .audio_codec = V4L2_MPEG_AUDIO_ENCODING_AAC, + /* original picture controls for firmware version <= 0x15 */ + /* updated in device_authorization() for newer firmware */ .brightness = 0x86, .contrast = 0x80, .hue = 0x80, diff --git a/drivers/media/video/hdpvr/hdpvr-video.c b/drivers/media/video/hdpvr/hdpvr-video.c index 087f7c0..36bb057 100644 --- a/drivers/media/video/hdpvr/hdpvr-video.c +++ b/drivers/media/video/hdpvr/hdpvr-video.c @@ -722,21 +722,39 @@ static const s32 supported_v4l2_ctrls[] = { }; static int fill_queryctrl(struct hdpvr_options *opt, struct v4l2_queryctrl *qc, - int ac3) + int ac3, int fw_ver) { int err; + if (fw_ver > 0x15) { + switch (qc->id) { + case V4L2_CID_BRIGHTNESS: + return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); + case V4L2_CID_CONTRAST: + return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x40); + case V4L2_CID_SATURATION: + return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x40); + case V4L2_CID_HUE: + return v4l2_ctrl_query_fill(qc, 0x0, 0x1e, 1, 0xf); + case V4L2_CID_SHARPNESS: + return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); + } + } else { + switch (qc->id) { + case V4L2_CID_BRIGHTNESS: + return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x86); + case V4L2_CID_CONTRAST: + return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); + case V4L2_CID_SATURATION: + return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); + case V4L2_CID_HUE: + return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); + case V4L2_CID_SHARPNESS: + return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); + } + } + switch (qc->id) { - case V4L2_CID_BRIGHTNESS: - return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x86); - case V4L2_CID_CONTRAST: - return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); - case V4L2_CID_SATURATION: - return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); - case V4L2_CID_HUE: - return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); - case V4L2_CID_SHARPNESS: - return v4l2_ctrl_query_fill(qc, 0x0, 0xff, 1, 0x80); case V4L2_CID_MPEG_AUDIO_ENCODING: return v4l2_ctrl_query_fill( qc, V4L2_MPEG_AUDIO_ENCODING_AAC, @@ -794,7 +812,8 @@ static int vidioc_queryctrl(struct file *file, void *private_data, if (qc->id == supported_v4l2_ctrls[i]) return fill_queryctrl(&dev->options, qc, - dev->flags & HDPVR_FLAG_AC3_CAP); + dev->flags & HDPVR_FLAG_AC3_CAP, + dev->fw_ver); if (qc->id < supported_v4l2_ctrls[i]) break; diff --git a/drivers/media/video/hdpvr/hdpvr.h b/drivers/media/video/hdpvr/hdpvr.h index d6439db..fea3c69 100644 --- a/drivers/media/video/hdpvr/hdpvr.h +++ b/drivers/media/video/hdpvr/hdpvr.h @@ -113,6 +113,7 @@ struct hdpvr_device { /* usb control transfer buffer and lock */ struct mutex usbc_mutex; u8 *usbc_buf; + u8 fw_ver; }; static inline struct hdpvr_device *to_hdpvr_dev(struct v4l2_device *v4l2_dev)