diff mbox series

[08/11] drm/meson: Add G12A support for CVBS Encoer

Message ID 20190325141824.21259-9-narmstrong@baylibre.com (mailing list archive)
State Not Applicable
Delegated to: Neil Armstrong
Headers show
Series drm/meson: Add G12A Support | expand

Commit Message

Neil Armstrong March 25, 2019, 2:18 p.m. UTC
The Meson G12A SoCs uses the exact same CVBS encoder except a simple
CVBS DAC register offset and settings delta.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/meson/meson_venc.c      | 11 +++++++++--
 drivers/gpu/drm/meson/meson_venc_cvbs.c | 25 ++++++++++++++++++-------
 2 files changed, 27 insertions(+), 9 deletions(-)

Comments

Jerome Brunet April 9, 2019, 8:43 a.m. UTC | #1
On Mon, 2019-03-25 at 15:18 +0100, Neil Armstrong wrote:
> The Meson G12A SoCs uses the exact same CVBS encoder except a simple
> CVBS DAC register offset and settings delta.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/meson/meson_venc.c      | 11 +++++++++--
>  drivers/gpu/drm/meson/meson_venc_cvbs.c | 25 ++++++++++++++++++-------
>  2 files changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
> index 66d73a932d19..6faca7313339 100644
> --- a/drivers/gpu/drm/meson/meson_venc.c
> +++ b/drivers/gpu/drm/meson/meson_venc.c
> @@ -73,7 +73,9 @@
>  /* HHI Registers */
>  #define HHI_GCLK_MPEG2		0x148 /* 0x52 offset in data sheet */
>  #define HHI_VDAC_CNTL0		0x2F4 /* 0xbd offset in data sheet */
> +#define HHI_VDAC_CNTL0_G12A	0x2EC /* 0xbd offset in data sheet */
>  #define HHI_VDAC_CNTL1		0x2F8 /* 0xbe offset in data sheet */
> +#define HHI_VDAC_CNTL1_G12A	0x2F0 /* 0xbe offset in data sheet */
>  #define HHI_HDMI_PHY_CNTL0	0x3a0 /* 0xe8 offset in data sheet */
>  
>  struct meson_cvbs_enci_mode meson_cvbs_enci_pal = {
> @@ -1675,8 +1677,13 @@ void meson_venc_disable_vsync(struct meson_drm *priv)
>  void meson_venc_init(struct meson_drm *priv)
>  {
>  	/* Disable CVBS VDAC */
> -	regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
> -	regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
> +	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 8);
> +	} else {
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
> +	}
>  
>  	/* Power Down Dacs */
>  	writel_relaxed(0xff, priv->io_base + _REG(VENC_VDAC_SETTING));
> diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c b/drivers/gpu/drm/meson/meson_venc_cvbs.c
> index d622d817b6df..2c5341c881c4 100644
> --- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
> +++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c
> @@ -37,7 +37,9 @@
>  
>  /* HHI VDAC Registers */
>  #define HHI_VDAC_CNTL0		0x2F4 /* 0xbd offset in data sheet */
> +#define HHI_VDAC_CNTL0_G12A	0x2EC /* 0xbd offset in data sheet */
>  #define HHI_VDAC_CNTL1		0x2F8 /* 0xbe offset in data sheet */
> +#define HHI_VDAC_CNTL1_G12A	0x2F0 /* 0xbe offset in data sheet */
>  
>  struct meson_venc_cvbs {
>  	struct drm_encoder	encoder;
> @@ -166,8 +168,13 @@ static void meson_venc_cvbs_encoder_disable(struct drm_encoder *encoder)
>  	struct meson_drm *priv = meson_venc_cvbs->priv;
>  
>  	/* Disable CVBS VDAC */
> -	regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
> -	regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
> +	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
> +	} else {
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);

I imagine 8 stands for BIT(3) ? Could you add a define to explain (quickly)
what it is ?

> +	}
>  }
>  
>  static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
> @@ -179,13 +186,17 @@ static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
>  	/* VDAC0 source is not from ATV */
>  	writel_bits_relaxed(BIT(5), 0, priv->io_base + _REG(VENC_VDAC_DACSEL0));
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> +	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 1);
> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
> +	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> +		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0xf0001);
> -
> -	regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
> +	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0x906001);
> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
> +	}

Maybe the values above are just magics taken from the vendor kernel, but if
you can, it would be nice to break them down to help us understand what is
controlled in these CTNL registers.

>  }
>  
>  static void meson_venc_cvbs_encoder_mode_set(struct drm_encoder *encoder,
Jerome Brunet April 9, 2019, 8:44 a.m. UTC | #2
On Tue, 2019-04-09 at 10:43 +0200, Jerome Brunet wrote:
> On Mon, 2019-03-25 at 15:18 +0100, Neil Armstrong wrote:
> > The Meson G12A SoCs uses the exact same CVBS encoder except a simple
> > CVBS DAC register offset and settings delta.
> > 
> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > ---
> >   drivers/gpu/drm/meson/meson_venc.c      | 11 +++++++++--
> >   drivers/gpu/drm/meson/meson_venc_cvbs.c | 25 ++++++++++++++++++-------
> >   2 files changed, 27 insertions(+), 9 deletions(-)

... and there is a typo in the patch title. s/Encoer/Encoder
Neil Armstrong April 9, 2019, 9:18 a.m. UTC | #3
On 09/04/2019 10:43, Jerome Brunet wrote:
> On Mon, 2019-03-25 at 15:18 +0100, Neil Armstrong wrote:
>> The Meson G12A SoCs uses the exact same CVBS encoder except a simple
>> CVBS DAC register offset and settings delta.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/gpu/drm/meson/meson_venc.c      | 11 +++++++++--
>>  drivers/gpu/drm/meson/meson_venc_cvbs.c | 25 ++++++++++++++++++-------
>>  2 files changed, 27 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
>> index 66d73a932d19..6faca7313339 100644
>> --- a/drivers/gpu/drm/meson/meson_venc.c
>> +++ b/drivers/gpu/drm/meson/meson_venc.c
>> @@ -73,7 +73,9 @@
>>  /* HHI Registers */
>>  #define HHI_GCLK_MPEG2		0x148 /* 0x52 offset in data sheet */
>>  #define HHI_VDAC_CNTL0		0x2F4 /* 0xbd offset in data sheet */
>> +#define HHI_VDAC_CNTL0_G12A	0x2EC /* 0xbd offset in data sheet */
>>  #define HHI_VDAC_CNTL1		0x2F8 /* 0xbe offset in data sheet */
>> +#define HHI_VDAC_CNTL1_G12A	0x2F0 /* 0xbe offset in data sheet */
>>  #define HHI_HDMI_PHY_CNTL0	0x3a0 /* 0xe8 offset in data sheet */
>>  
>>  struct meson_cvbs_enci_mode meson_cvbs_enci_pal = {
>> @@ -1675,8 +1677,13 @@ void meson_venc_disable_vsync(struct meson_drm *priv)
>>  void meson_venc_init(struct meson_drm *priv)
>>  {
>>  	/* Disable CVBS VDAC */
>> -	regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
>> -	regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
>> +	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 8);
>> +	} else {
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
>> +	}
>>  
>>  	/* Power Down Dacs */
>>  	writel_relaxed(0xff, priv->io_base + _REG(VENC_VDAC_SETTING));
>> diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c b/drivers/gpu/drm/meson/meson_venc_cvbs.c
>> index d622d817b6df..2c5341c881c4 100644
>> --- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
>> +++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c
>> @@ -37,7 +37,9 @@
>>  
>>  /* HHI VDAC Registers */
>>  #define HHI_VDAC_CNTL0		0x2F4 /* 0xbd offset in data sheet */
>> +#define HHI_VDAC_CNTL0_G12A	0x2EC /* 0xbd offset in data sheet */
>>  #define HHI_VDAC_CNTL1		0x2F8 /* 0xbe offset in data sheet */
>> +#define HHI_VDAC_CNTL1_G12A	0x2F0 /* 0xbe offset in data sheet */
>>  
>>  struct meson_venc_cvbs {
>>  	struct drm_encoder	encoder;
>> @@ -166,8 +168,13 @@ static void meson_venc_cvbs_encoder_disable(struct drm_encoder *encoder)
>>  	struct meson_drm *priv = meson_venc_cvbs->priv;
>>  
>>  	/* Disable CVBS VDAC */
>> -	regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
>> -	regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
>> +	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
>> +	} else {
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
> 
> I imagine 8 stands for BIT(3) ? Could you add a define to explain (quickly)
> what it is ?

Ack

> 
>> +	}
>>  }
>>  
>>  static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
>> @@ -179,13 +186,17 @@ static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
>>  	/* VDAC0 source is not from ATV */
>>  	writel_bits_relaxed(BIT(5), 0, priv->io_base + _REG(VENC_VDAC_DACSEL0));
>>  
>> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
>> +	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
>>  		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 1);
>> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
>> -		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
>> +	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
>> +		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
>>  		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0xf0001);
>> -
>> -	regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
>> +	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0x906001);
>> +		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
>> +	}
> 
> Maybe the values above are just magics taken from the vendor kernel, but if
> you can, it would be nice to break them down to help us understand what is
> controlled in these CTNL registers.

It's pretty magic values, but I'll do my best.

> 
>>  }
>>  
>>  static void meson_venc_cvbs_encoder_mode_set(struct drm_encoder *encoder,
> 
> 
Will fix in a follow-up patch,

Neil
Neil Armstrong April 9, 2019, 9:18 a.m. UTC | #4
On 09/04/2019 10:44, Jerome Brunet wrote:
> On Tue, 2019-04-09 at 10:43 +0200, Jerome Brunet wrote:
>> On Mon, 2019-03-25 at 15:18 +0100, Neil Armstrong wrote:
>>> The Meson G12A SoCs uses the exact same CVBS encoder except a simple
>>> CVBS DAC register offset and settings delta.
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>>   drivers/gpu/drm/meson/meson_venc.c      | 11 +++++++++--
>>>   drivers/gpu/drm/meson/meson_venc_cvbs.c | 25 ++++++++++++++++++-------
>>>   2 files changed, 27 insertions(+), 9 deletions(-)
> 
> ... and there is a typo in the patch title. s/Encoer/Encoder
> 

Thanks for pointing this, I'll fix while applying.

Neil
diff mbox series

Patch

diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
index 66d73a932d19..6faca7313339 100644
--- a/drivers/gpu/drm/meson/meson_venc.c
+++ b/drivers/gpu/drm/meson/meson_venc.c
@@ -73,7 +73,9 @@ 
 /* HHI Registers */
 #define HHI_GCLK_MPEG2		0x148 /* 0x52 offset in data sheet */
 #define HHI_VDAC_CNTL0		0x2F4 /* 0xbd offset in data sheet */
+#define HHI_VDAC_CNTL0_G12A	0x2EC /* 0xbd offset in data sheet */
 #define HHI_VDAC_CNTL1		0x2F8 /* 0xbe offset in data sheet */
+#define HHI_VDAC_CNTL1_G12A	0x2F0 /* 0xbe offset in data sheet */
 #define HHI_HDMI_PHY_CNTL0	0x3a0 /* 0xe8 offset in data sheet */
 
 struct meson_cvbs_enci_mode meson_cvbs_enci_pal = {
@@ -1675,8 +1677,13 @@  void meson_venc_disable_vsync(struct meson_drm *priv)
 void meson_venc_init(struct meson_drm *priv)
 {
 	/* Disable CVBS VDAC */
-	regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
-	regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
+	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
+		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 8);
+	} else {
+		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
+		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
+	}
 
 	/* Power Down Dacs */
 	writel_relaxed(0xff, priv->io_base + _REG(VENC_VDAC_SETTING));
diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c b/drivers/gpu/drm/meson/meson_venc_cvbs.c
index d622d817b6df..2c5341c881c4 100644
--- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c
@@ -37,7 +37,9 @@ 
 
 /* HHI VDAC Registers */
 #define HHI_VDAC_CNTL0		0x2F4 /* 0xbd offset in data sheet */
+#define HHI_VDAC_CNTL0_G12A	0x2EC /* 0xbd offset in data sheet */
 #define HHI_VDAC_CNTL1		0x2F8 /* 0xbe offset in data sheet */
+#define HHI_VDAC_CNTL1_G12A	0x2F0 /* 0xbe offset in data sheet */
 
 struct meson_venc_cvbs {
 	struct drm_encoder	encoder;
@@ -166,8 +168,13 @@  static void meson_venc_cvbs_encoder_disable(struct drm_encoder *encoder)
 	struct meson_drm *priv = meson_venc_cvbs->priv;
 
 	/* Disable CVBS VDAC */
-	regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
-	regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
+	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
+		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
+	} else {
+		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0);
+		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 8);
+	}
 }
 
 static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
@@ -179,13 +186,17 @@  static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
 	/* VDAC0 source is not from ATV */
 	writel_bits_relaxed(BIT(5), 0, priv->io_base + _REG(VENC_VDAC_DACSEL0));
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
+	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
 		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 1);
-	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
+		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
+	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
+		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
 		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0xf0001);
-
-	regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
+		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
+	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0x906001);
+		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
+	}
 }
 
 static void meson_venc_cvbs_encoder_mode_set(struct drm_encoder *encoder,