From 1cd04c75ca99803aa0a1bd9b4952f2d61e72c4f4 Mon Sep 17 00:00:00 2001 From: Mike Isely Date: Sun, 25 Nov 2007 22:48:52 -0600 Subject: pvrusb2: Rework pipeline state control From: Mike Isely This is a new implementation for video pipeline control within the pvrusb2 driver. Actual start/stop of the pipeline is moved to the driver's kernel thread. Pipeline stages are controlled autonomously based on surrounding pipeline or application control state. Kernel thread management is also cleaned up and moved into the internal control structure of the driver, solving a set up / tear down race along the way. Better failure recovery is implemented with this new control strategy. Also with this change comes better control of the cx23416 encoder, building on additional information learned about the peculiarities of controlling this part (this information was the original trigger for this rework). With this change, overall encoder stability should be considerably improved. Yes, this is a large change for this driver, but due to the nature of the feature being worked on, the changes are fairly pervasive and would be difficult to break into smaller pieces with any semblence of step-wise stability. Signed-off-by: Mike Isely --- .../media/video/pvrusb2/pvrusb2-hdw-internal.h | 64 +++++++++++++++++----- 1 file changed, 50 insertions(+), 14 deletions(-) (limited to 'linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h') diff --git a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h index fbe5d46d6..70ed3eef2 100644 --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h @@ -35,6 +35,7 @@ #include #include +#include #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,16) #include #else @@ -183,6 +184,12 @@ struct pvr2_hdw { /* Device type, one of PVR2_HDW_TYPE_xxxxx */ unsigned int hdw_type; + /* Kernel worker thread handling */ + struct workqueue_struct *workqueue; + struct work_struct workpoll; /* Update driver state */ + struct work_struct worki2csync; /* Update i2c clients */ + struct work_struct workinit; /* Driver initialization sequence */ + /* Video spigot */ struct pvr2_stream *vid_stream; @@ -194,9 +201,6 @@ struct pvr2_hdw { #endif int big_lock_held; /* For debugging */ - void (*poll_trigger_func)(void *); - void *poll_trigger_data; - char name[32]; /* I2C stuff */ @@ -241,14 +245,48 @@ struct pvr2_hdw { unsigned int cmd_debug_write_len; // unsigned int cmd_debug_read_len; // + /* Bits of state that describe what is going on with various parts + of the driver. */ + volatile int state_encoder_ok; /* Encoder is operational */ + volatile int state_encoder_run; /* Encoder is running */ + volatile int state_encoder_config; /* Encoder is configured */ + volatile int state_encoder_waitok; /* Encoder pre-wait done */ + volatile int state_decoder_run; /* Decoder is running */ + volatile int state_usbstream_run; /* FX2 is streaming */ + volatile int state_decoder_quiescent; /* Decoder idle for > 50msec */ + volatile int state_pipeline_config; /* Pipeline is configured */ + int state_pipeline_req; /* Somebody wants to stream */ + int state_pipeline_pause; /* Pipeline must be paused */ + int state_pipeline_idle; /* Pipeline not running */ + + /* This is the master state of the driver. It is the combined + result of other bits of state. Examining this will indicate the + overall state of the driver. Values here are one of + PVR2_STATE_xxxx */ + unsigned int master_state; + + /* True if states must be re-evaluated */ + int state_stale; + + void (*state_func)(void *); + void *state_data; + + /* Timer for measuring decoder settling time */ + struct timer_list quiescent_timer; + + /* Timer for measuring encoder pre-wait time */ + struct timer_list encoder_wait_timer; + + /* Place to block while waiting for state changes */ + wait_queue_head_t state_wait_data; + + int flag_ok; /* device in known good state */ int flag_disconnected; /* flag_ok == 0 due to disconnect */ int flag_init_ok; /* true if structure is fully initialized */ - int flag_streaming_enabled; /* true if streaming should be on */ int fw1_state; /* current situation with fw1 */ - int flag_encoder_ok; /* True if encoder is healthy */ - - int flag_decoder_is_tuned; + int flag_decoder_missed;/* We've noticed missing decoder */ + int flag_tripped; /* Indicates overall failure to start */ struct pvr2_decoder_ctrl *decoder_ctrl; @@ -257,12 +295,6 @@ struct pvr2_hdw { unsigned int fw_size; int fw_cpu_flag; /* True if we are dealing with the CPU */ - // Which subsystem pieces have been enabled / configured - unsigned long subsys_enabled_mask; - - // Which subsystems are manipulated to enable streaming - unsigned long subsys_stream_mask; - // True if there is a request to trigger logging of state in each // module. int log_requested; @@ -312,13 +344,16 @@ struct pvr2_hdw { /* Location of eeprom or a negative number if none */ int eeprom_addr; - enum pvr2_config config; + enum pvr2_config active_stream_type; + enum pvr2_config desired_stream_type; /* Control state needed for cx2341x module */ struct cx2341x_mpeg_params enc_cur_state; struct cx2341x_mpeg_params enc_ctl_state; /* True if an encoder attribute has changed */ int enc_stale; + /* True if an unsafe encoder attribute has changed */ + int enc_unsafe_stale; /* True if enc_cur_state is valid */ int enc_cur_valid; @@ -348,6 +383,7 @@ struct pvr2_hdw { /* This function gets the current frequency */ unsigned long pvr2_hdw_get_cur_freq(struct pvr2_hdw *); +void pvr2_hdw_set_decoder(struct pvr2_hdw *,struct pvr2_decoder_ctrl *); #endif /* __PVRUSB2_HDW_INTERNAL_H */ -- cgit v1.2.3 From 2446fc6debfd93e7b8b9cad4b22083994f686e59 Mon Sep 17 00:00:00 2001 From: Mike Isely Date: Sun, 25 Nov 2007 22:53:12 -0600 Subject: pvrusb2: Centralize device specific attributes into a single place. From: Mike Isely The pvrusb2 driver currently supports two variants of the Hauppauge PVR USB2. However there are other hardware types potentially supportable, but the driver at the moment is not structured to make it easy to describe these minor variations. This changeset is the first set of changes to make such additional device support possible. Device attributes are held in several tables all contained within pvrusb2-devattr.c; all other device-specific driver behavior now derives from these tables. Signed-off-by: Mike Isely --- linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h') diff --git a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h index 70ed3eef2..cd06005ae 100644 --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h @@ -44,6 +44,7 @@ #include "pvrusb2-hdw.h" #include "pvrusb2-io.h" #include +#include "pvrusb2-devattr.h" /* Legal values for PVR2_CID_HSM */ #define PVR2_CVAL_HSM_FAIL 0 @@ -166,10 +167,6 @@ struct pvr2_decoder_ctrl { #define FW1_STATE_RELOAD 3 #define FW1_STATE_OK 4 -/* Known major hardware variants, keyed from device ID */ -#define PVR2_HDW_TYPE_29XXX 0 -#define PVR2_HDW_TYPE_24XXX 1 - typedef int (*pvr2_i2c_func)(struct pvr2_hdw *,u8,u8 *,u16,u8 *, u16); #define PVR2_I2C_FUNC_CNT 128 @@ -183,6 +180,7 @@ struct pvr2_hdw { /* Device type, one of PVR2_HDW_TYPE_xxxxx */ unsigned int hdw_type; + const struct pvr2_device_desc *hdw_desc; /* Kernel worker thread handling */ struct workqueue_struct *workqueue; -- cgit v1.2.3 From aad9eaebf68a27da1d3d889b885379cd7e27cba0 Mon Sep 17 00:00:00 2001 From: Mike Isely Date: Sun, 25 Nov 2007 22:55:07 -0600 Subject: pvrusb2: Remove obsolete global hardware type enumeration From: Mike Isely Device-specific driver behavior is now defined by generic device characteristics rather than by specific device model information. With this change, the hardware type field can go away, thus this change. Signed-off-by: Mike Isely --- linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h') diff --git a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h index cd06005ae..a8e829d37 100644 --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h @@ -178,8 +178,8 @@ struct pvr2_hdw { struct usb_device *usb_dev; struct usb_interface *usb_intf; - /* Device type, one of PVR2_HDW_TYPE_xxxxx */ - unsigned int hdw_type; + /* Device description, anything that must adjust behavior based on + device specific info will use information held here. */ const struct pvr2_device_desc *hdw_desc; /* Kernel worker thread handling */ -- cgit v1.2.3 From 44549c9cceb63ac4b50f950c361ecb2bea79bce2 Mon Sep 17 00:00:00 2001 From: Mike Isely Date: Sun, 2 Dec 2007 22:43:23 -0600 Subject: pvrusb2: Remove use of volatile in command sequencer From: Mike Isely pvrusb2: Remove use of volatile for command sequencer; these variables are set by interrupt-context code and we check their state in such a manner that there should be no race conditions. This had originally been done out of paranoia, and I have since been convinced that the paranoia is not required. Signed-off-by: Mike Isely --- linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h') diff --git a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h index a8e829d37..ff99ed5fa 100644 --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h @@ -233,9 +233,9 @@ struct pvr2_hdw { struct urb *ctl_read_urb; unsigned char *ctl_write_buffer; unsigned char *ctl_read_buffer; - volatile int ctl_write_pend_flag; - volatile int ctl_read_pend_flag; - volatile int ctl_timeout_flag; + int ctl_write_pend_flag; + int ctl_read_pend_flag; + int ctl_timeout_flag; struct completion ctl_done; unsigned char cmd_buffer[PVR2_CTL_BUFFSIZE]; int cmd_debug_state; // Low level command debugging info -- cgit v1.2.3 From e39a15b8e87d62fd7bc1974bd27012d80048ddef Mon Sep 17 00:00:00 2001 From: Mike Isely Date: Sun, 2 Dec 2007 22:44:43 -0600 Subject: pvrusb2: Remove use of volatile in pipeline control state machine From: Mike Isely pvrusb2: Eliminate use of volatile in pipeline control state variables. These were all cases of paranoia; upon further review the overall mechanism employed here should not require use of volatile. This had originally been done out of paranoia, and I have since been convinced that the paranoia is not required. Signed-off-by: Mike Isely --- linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h') diff --git a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h index ff99ed5fa..dfaa347b1 100644 --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h @@ -245,14 +245,14 @@ struct pvr2_hdw { /* Bits of state that describe what is going on with various parts of the driver. */ - volatile int state_encoder_ok; /* Encoder is operational */ - volatile int state_encoder_run; /* Encoder is running */ - volatile int state_encoder_config; /* Encoder is configured */ - volatile int state_encoder_waitok; /* Encoder pre-wait done */ - volatile int state_decoder_run; /* Decoder is running */ - volatile int state_usbstream_run; /* FX2 is streaming */ - volatile int state_decoder_quiescent; /* Decoder idle for > 50msec */ - volatile int state_pipeline_config; /* Pipeline is configured */ + int state_encoder_ok; /* Encoder is operational */ + int state_encoder_run; /* Encoder is running */ + int state_encoder_config; /* Encoder is configured */ + int state_encoder_waitok; /* Encoder pre-wait done */ + int state_decoder_run; /* Decoder is running */ + int state_usbstream_run; /* FX2 is streaming */ + int state_decoder_quiescent; /* Decoder idle for > 50msec */ + int state_pipeline_config; /* Pipeline is configured */ int state_pipeline_req; /* Somebody wants to stream */ int state_pipeline_pause; /* Pipeline must be paused */ int state_pipeline_idle; /* Pipeline not running */ -- cgit v1.2.3