Re: [PATCH 05/11] media: adv7180: init chip with AD recommended register settings

[Lars-Peter Clausen]

On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
> Define and load register tables that conform to Analog Device's
> recommended register settings. It loads the default single-ended
> CVBS on Ain1 configuration for both ADV7180 and ADV7182 chips.

The driver should already setup the recommended configuration based on the
current mode. This patch seems to add a set of register writes that only
apply for a specific mode.

[...]
> Note this patch also enables NEWAVMODE, which is also recommended by
> Analog Devices. This will likely break any current backends using this
> subdev that are expecting different or manually configured AV codes.
> 
> Note also that bt.656-4 support has been removed in this patch, but it
> will be brought back in a subsequent patch.


I'm not sure if we can or should break backwards compatibility in this way.

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/11] media: adv7180: init chip with AD recommended register settings

[Tim Harvey]

On Wed, Jul 6, 2016 at 3:59 PM, Steve Longerbeam  wrote:
> Define and load register tables that conform to Analog Device's
> recommended register settings. It loads the default single-ended
> CVBS on Ain1 configuration for both ADV7180 and ADV7182 chips.
>
> New register addresses have been defined for the tables. Those new
> defines are also used in existing locations where hard-coded addresses
> were used.
>
> Note this patch also enables NEWAVMODE, which is also recommended by
> Analog Devices. This will likely break any current backends using this
> subdev that are expecting different or manually configured AV codes.
>
> Note also that bt.656-4 support has been removed in this patch, but it
> will be brought back in a subsequent patch.
>
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/media/i2c/adv7180.c | 168 
> ++--
>  1 file changed, 130 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 42816d4..92e2f37 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -56,10 +56,11 @@
>
>  #define ADV7182_REG_INPUT_VIDSEL   0x0002
>
> +#define ADV7180_REG_OUTPUT_CONTROL 0x0003
>  #define ADV7180_REG_EXTENDED_OUTPUT_CONTROL0x0004
>  #define ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS0xC5
>
> -#define ADV7180_REG_AUTODETECT_ENABLE  0x07
> +#define ADV7180_REG_AUTODETECT_ENABLE  0x0007
>  #define ADV7180_AUTODETECT_DEFAULT 0x7f
>  /* Contrast */
>  #define ADV7180_REG_CON0x0008  /*Unsigned */
> @@ -100,6 +101,20 @@
>  #define ADV7180_REG_IDENT 0x0011
>  #define ADV7180_ID_7180 0x18
>
> +#define ADV7180_REG_STATUS30x0013
> +#define ADV7180_REG_ANALOG_CLAMP_CTL   0x0014
> +#define ADV7180_REG_SHAP_FILTER_CTL_1  0x0017
> +#define ADV7180_REG_CTRL_2 0x001d
> +#define ADV7180_REG_VSYNC_FIELD_CTL_1  0x0031
> +#define ADV7180_REG_MANUAL_WIN_CTL_1   0x003d
> +#define ADV7180_REG_MANUAL_WIN_CTL_2   0x003e
> +#define ADV7180_REG_MANUAL_WIN_CTL_3   0x003f
> +#define ADV7180_REG_LOCK_CNT   0x0051
> +#define ADV7180_REG_CVBS_TRIM  0x0052
> +#define ADV7180_REG_CLAMP_ADJ  0x005a
> +#define ADV7180_REG_RES_CIR0x005f
> +#define ADV7180_REG_DIFF_MODE  0x0060
> +
>  #define ADV7180_REG_ICONF1 0x2040
>  #define ADV7180_ICONF1_ACTIVE_LOW  0x01
>  #define ADV7180_ICONF1_PSYNC_ONLY  0x10
> @@ -129,9 +144,15 @@
>  #define ADV7180_REG_VPP_SLAVE_ADDR 0xFD
>  #define ADV7180_REG_CSI_SLAVE_ADDR 0xFE
>
> -#define ADV7180_REG_FLCONTROL 0x40e0
> +#define ADV7180_REG_ACE_CTRL1  0x4080
> +#define ADV7180_REG_ACE_CTRL5  0x4084
> +#define ADV7180_REG_FLCONTROL  0x40e0
>  #define ADV7180_FLCONTROL_FL_ENABLE 0x1
>
> +#define ADV7180_REG_RST_CLAMP  0x809c
> +#define ADV7180_REG_AGC_ADJ1   0x80b6
> +#define ADV7180_REG_AGC_ADJ2   0x80c0
> +
>  #define ADV7180_CSI_REG_PWRDN  0x00
>  #define ADV7180_CSI_PWRDN  0x80
>
> @@ -209,6 +230,11 @@ struct adv7180_state {
> struct adv7180_state,   \
> ctrl_hdl)->sd)
>
> +struct adv7180_reg_tbl_t {
> +   unsigned int reg;
> +   unsigned int val;
> +};
> +
>  static int adv7180_select_page(struct adv7180_state *state, unsigned int 
> page)
>  {
> if (state->register_page != page) {
> @@ -235,6 +261,20 @@ static int adv7180_read(struct adv7180_state *state, 
> unsigned int reg)
> return i2c_smbus_read_byte_data(state->client, reg & 0xff);
>  }
>
> +static int adv7180_load_reg_tbl(struct adv7180_state *state,
> +   const struct adv7180_reg_tbl_t *tbl, int n)
> +{
> +   int ret, i;
> +
> +   for (i = 0; i < n; i++) {
> +   ret = adv7180_write(state, tbl[i].reg, tbl[i].val);
> +   if (ret)
> +   return ret;
> +   }
> +
> +   return 0;
> +}
> +
>  static int adv7180_csi_write(struct adv7180_state *state, unsigned int reg,
> unsigned int value)
>  {
> @@ -828,19 +868,36 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
> return IRQ_HANDLED;
>  }
>
> +/*
> + * This register table conforms to Analog Device's Register Settings
> + * Recommendation for the ADV7180. It configures single-ended CVBS
> + * input on Ain1, and enables NEWAVMODE.
> + */
> +static const struct adv7180_reg_tbl_t adv7180_single_ended_cvbs[] = {
> +   /* Set analog mux for CVBS on Ain1 */
> +   { ADV7180_REG_INPUT_CONTROL, 0x00 },
> +   /* ADI Required Write: Reset Clamp Circuitry */
> +   { ADV7180_REG_ANALOG_CLAMP_CTL, 0x30 },
> +   /* Enable SFL Output */
> +   { ADV7180_REG_EXTENDED_OUTPUT_CONTROL, 0x57 },
> +   /* Select SH1 Chroma Shaping Filter */
> +