[PATCH 0/2] drm/sun4i: fix DCLK and improve its handling

[PATCH 0/2] drm/sun4i: fix DCLK and improve its handling

[Giulio Benetti]

First patch is a tested by me fix, while the second need testing to
understand if it works correctly with any sunxi SoC with DE peripheral.
Already tested SoCs are:
- A20
- A33

Need testing:
- A10
- A10s
- A13

Giulio Benetti (2):
  drm/sun4i: tcon: fix inverted DCLK polarity
  drm/sun4i: tcon: improve DCLK polarity handling

 drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
 2 files changed, 2 insertions(+), 19 deletions(-)

-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/sun4i: tcon: fix inverted DCLK polarity

[Giulio Benetti]

During commit "88bc4178568b8e0331143cc0616640ab72f0cba1" DRM_BUS_FLAG_*
macros have been changed to avoid ambiguity but just because of this
ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
_SAMPLE_ not _DRIVE_. This lead to DLCK inversion, so let's swap DCLK
phase to fix it.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index eaaf5d70e352..52598bb0fb0b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -585,10 +585,10 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	 * and DOTCLOCK drivers.
 	 */
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
-		clk_set_phase(tcon->dclk, 240);
+		clk_set_phase(tcon->dclk, 0);
 
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
-		clk_set_phase(tcon->dclk, 0);
+		clk_set_phase(tcon->dclk, 240);
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
 			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/sun4i: tcon: fix inverted DCLK polarity

[Jernej Škrabec]

Dne sreda, 06. januar 2021 ob 20:27:59 CET je Giulio Benetti napisal(a):
> During commit "88bc4178568b8e0331143cc0616640ab72f0cba1" DRM_BUS_FLAG_*

Please use same commit referencing approach as for "Fixes" tag.

> macros have been changed to avoid ambiguity but just because of this
> ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
> _SAMPLE_ not _DRIVE_. This lead to DLCK inversion, so let's swap DCLK
> phase to fix it.
> 

Add Fixes tag here.

Best regards,
Jernej

> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/
sun4i_tcon.c
> index eaaf5d70e352..52598bb0fb0b 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -585,10 +585,10 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon 
*tcon,
>  	 * and DOTCLOCK drivers.
>  	 */
>  	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> -		clk_set_phase(tcon->dclk, 240);
> +		clk_set_phase(tcon->dclk, 0);
>  
>  	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> -		clk_set_phase(tcon->dclk, 0);
> +		clk_set_phase(tcon->dclk, 240);
>  
>  	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
>  			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
> -- 
> 2.25.1
> 
> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 0/2] drm/sun4i: fix DCLK and improve its handling

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti@micronovasrl.com>

First patch is a tested by me fix, while the second need testing to
understand if it works correctly with any sunxi SoC with DE peripheral.
Already tested SoCs are:
- A20
- A33

Need testing:
- A10
- A10s
- A13

Giulio Benetti (2):
  drm/sun4i: tcon: fix inverted DCLK polarity
  drm/sun4i: tcon: improve DCLK polarity handling

 drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
 2 files changed, 2 insertions(+), 19 deletions(-)

-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/2] drm/sun4i: tcon: fix inverted DCLK polarity

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti@micronovasrl.com>

During commit 88bc4178568b ("drm: Use new
DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
macros have been changed to avoid ambiguity but just because of this
ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
_SAMPLE_ not _DRIVE_. This lead to DLCK inversion, so let's swap DCLK
phase to fix it.

Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
V1->V2:
use Fixes: tag in commit log as suggested by Jernej
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index eaaf5d70e352..52598bb0fb0b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -585,10 +585,10 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	 * and DOTCLOCK drivers.
 	 */
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
-		clk_set_phase(tcon->dclk, 240);
+		clk_set_phase(tcon->dclk, 0);
 
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
-		clk_set_phase(tcon->dclk, 0);
+		clk_set_phase(tcon->dclk, 240);
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
 			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti@micronovasrl.com>

It turned out(Maxime suggestion) that bit 26 of SUN4I_TCON0_IO_POL_REG is
dedicated to invert DCLK polarity and this makes thing really easier than
before. So let's handle DCLK polarity by adding
SUN4I_TCON0_IO_POL_DCLK_POSITIVE as bit 26 and activating according to
bus_flags the same way is done for all the other signals.

Cc: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
 2 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 52598bb0fb0b..30171ccd87e5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
 		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
 
-	/*
-	 * On A20 and similar SoCs, the only way to achieve Positive Edge
-	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
-	 * By default TCON works in Negative Edge(Falling Edge),
-	 * this is why phase is set to 0 in that case.
-	 * Unfortunately there's no way to logically invert dclk through
-	 * IO_POL register.
-	 * The only acceptable way to work, triple checked with scope,
-	 * is using clock phase set to 0° for Negative Edge and set to 240°
-	 * for Positive Edge.
-	 * On A33 and similar SoCs there would be a 90° phase option,
-	 * but it divides also dclk by 2.
-	 * Following code is a way to avoid quirks all around TCON
-	 * and DOTCLOCK drivers.
-	 */
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
-		clk_set_phase(tcon->dclk, 0);
-
-	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
-		clk_set_phase(tcon->dclk, 240);
+		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
 			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index cfbf4e6c1679..0ce71d10a31b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -113,6 +113,7 @@
 #define SUN4I_TCON0_IO_POL_REG			0x88
 #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
 #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
+#define SUN4I_TCON0_IO_POL_DCLK_POSITIVE		BIT(26)
 #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
 #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
 
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling

[Maxime Ripard]

Hi,

Thanks for those patches

On Thu, Jan 07, 2021 at 03:30:32AM +0100, Giulio Benetti wrote:
> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
> 
> It turned out(Maxime suggestion) that bit 26 of SUN4I_TCON0_IO_POL_REG is
> dedicated to invert DCLK polarity and this makes thing really easier than
> before. So let's handle DCLK polarity by adding
> SUN4I_TCON0_IO_POL_DCLK_POSITIVE as bit 26 and activating according to
> bus_flags the same way is done for all the other signals.
> 
> Cc: Maxime Ripard <maxime@cerno.tech>

Suggested-by would be nice here :)

> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>  2 files changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 52598bb0fb0b..30171ccd87e5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>  	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>  		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>  
> -	/*
> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
> -	 * By default TCON works in Negative Edge(Falling Edge),
> -	 * this is why phase is set to 0 in that case.
> -	 * Unfortunately there's no way to logically invert dclk through
> -	 * IO_POL register.
> -	 * The only acceptable way to work, triple checked with scope,
> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
> -	 * for Positive Edge.
> -	 * On A33 and similar SoCs there would be a 90° phase option,
> -	 * but it divides also dclk by 2.
> -	 * Following code is a way to avoid quirks all around TCON
> -	 * and DOTCLOCK drivers.
> -	 */
>  	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> -		clk_set_phase(tcon->dclk, 0);
> -
> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> -		clk_set_phase(tcon->dclk, 240);
> +		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;

I'm not really sure why we need the first patch of this series here?
That patch only seem to undo what you did in patch 1

Maxime
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling

[Marjan Pascolo]

[-- Attachment #1.1: Type: text/plain, Size: 2924 bytes --]

Hi,

Quote "

I'm not really sure why we need the first patch of this series here?
That patch only seem to undo what you did in patch 1

"

And another question (probably could be a stupid one):

in "/[PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling/" I 
see you deleted:

-		clk_set_phase(tcon->dclk, 0);

Is safe to assume that phase register will be always set to 0?

Or maybe will be safer manually set it to 0 in every condition to avoid 
surprises (dirt values due to previous condition)?


Marjan

Il 08/01/2021 10:23, Maxime Ripard ha scritto:
> Hi,
>
> Thanks for those patches
>
> On Thu, Jan 07, 2021 at 03:30:32AM +0100, Giulio Benetti wrote:
>> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
>>
>> It turned out(Maxime suggestion) that bit 26 of SUN4I_TCON0_IO_POL_REG is
>> dedicated to invert DCLK polarity and this makes thing really easier than
>> before. So let's handle DCLK polarity by adding
>> SUN4I_TCON0_IO_POL_DCLK_POSITIVE as bit 26 and activating according to
>> bus_flags the same way is done for all the other signals.
>>
>> Cc: Maxime Ripard <maxime@cerno.tech>
> Suggested-by would be nice here :)
>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
>>   drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>>   2 files changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> index 52598bb0fb0b..30171ccd87e5 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> @@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>>   	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>>   		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>>   
>> -	/*
>> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
>> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
>> -	 * By default TCON works in Negative Edge(Falling Edge),
>> -	 * this is why phase is set to 0 in that case.
>> -	 * Unfortunately there's no way to logically invert dclk through
>> -	 * IO_POL register.
>> -	 * The only acceptable way to work, triple checked with scope,
>> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
>> -	 * for Positive Edge.
>> -	 * On A33 and similar SoCs there would be a 90° phase option,
>> -	 * but it divides also dclk by 2.
>> -	 * Following code is a way to avoid quirks all around TCON
>> -	 * and DOTCLOCK drivers.
>> -	 */
>>   	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
>> -		clk_set_phase(tcon->dclk, 0);
>> -
>> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>> -		clk_set_phase(tcon->dclk, 240);
>> +		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
> I'm not really sure why we need the first patch of this series here?
> That patch only seem to undo what you did in patch 1
>
> Maxime

[-- Attachment #1.2: Type: text/html, Size: 3985 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling

[Giulio Benetti]

Hi Marjan,

On 1/8/21 10:32 AM, Marjan Pascolo wrote:
> Hi,

please don't top post, answer in-line as we do, and please use 
plain-test instead of HTML. These are the standards in Mailing Lists(ML).

> Quote "
> 
> I'm not really sure why we need the first patch of this series here?
> That patch only seem to undo what you did in patch 1
> 
> "

Already answered to Maxime

> 
> And another question (probably could be a stupid one):
> 
> in "/[PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling/" I 
> see you deleted:
> 
> -		clk_set_phase(tcon->dclk, 0);
> 
> Is safe to assume that phase register will be always set to 0?

We always assumed it is set to 0 by default.

> 
> Or maybe will be safer manually set it to 0 in every condition to avoid 
> surprises (dirt values due to previous condition)?

That would be a good idea, so something like this:

'''
	int phase = 0;

	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
		phase = 240;

	clk_set_phase(tcon->dclk, phase);
'''

because now DRM_BUS_FLAG_PIXDATA_SAMPLE_ and DRM_BUS_FLAG_PIXDATA_DRIVE_ 
are exclusive, while before not.

But then if bit 26 solution works everything gets easier.

Best Regards
Giulio

> 
> Marjan
> 
> Il 08/01/2021 10:23, Maxime Ripard ha scritto:
>> Hi,
>>
>> Thanks for those patches
>>
>> On Thu, Jan 07, 2021 at 03:30:32AM +0100, Giulio Benetti wrote:
>>> From: Giulio Benetti<giulio.benetti@micronovasrl.com>
>>>
>>> It turned out(Maxime suggestion) that bit 26 of SUN4I_TCON0_IO_POL_REG is
>>> dedicated to invert DCLK polarity and this makes thing really easier than
>>> before. So let's handle DCLK polarity by adding
>>> SUN4I_TCON0_IO_POL_DCLK_POSITIVE as bit 26 and activating according to
>>> bus_flags the same way is done for all the other signals.
>>>
>>> Cc: Maxime Ripard<maxime@cerno.tech>
>> Suggested-by would be nice here :)
>>
>>> Signed-off-by: Giulio Benetti<giulio.benetti@micronovasrl.com>
>>> ---
>>>   drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
>>>   drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>>>   2 files changed, 2 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>> index 52598bb0fb0b..30171ccd87e5 100644
>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>> @@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>>>   	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>>>   		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>>>   
>>> -	/*
>>> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
>>> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
>>> -	 * By default TCON works in Negative Edge(Falling Edge),
>>> -	 * this is why phase is set to 0 in that case.
>>> -	 * Unfortunately there's no way to logically invert dclk through
>>> -	 * IO_POL register.
>>> -	 * The only acceptable way to work, triple checked with scope,
>>> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
>>> -	 * for Positive Edge.
>>> -	 * On A33 and similar SoCs there would be a 90° phase option,
>>> -	 * but it divides also dclk by 2.
>>> -	 * Following code is a way to avoid quirks all around TCON
>>> -	 * and DOTCLOCK drivers.
>>> -	 */
>>>   	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
>>> -		clk_set_phase(tcon->dclk, 0);
>>> -
>>> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>>> -		clk_set_phase(tcon->dclk, 240);
>>> +		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
>> I'm not really sure why we need the first patch of this series here?
>> That patch only seem to undo what you did in patch 1
>>
>> Maxime
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling

[Giulio Benetti]

Hi,

On 1/8/21 10:23 AM, Maxime Ripard wrote:
> Hi,
> 
> Thanks for those patches
> 
> On Thu, Jan 07, 2021 at 03:30:32AM +0100, Giulio Benetti wrote:
>> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
>>
>> It turned out(Maxime suggestion) that bit 26 of SUN4I_TCON0_IO_POL_REG is
>> dedicated to invert DCLK polarity and this makes thing really easier than
>> before. So let's handle DCLK polarity by adding
>> SUN4I_TCON0_IO_POL_DCLK_POSITIVE as bit 26 and activating according to
>> bus_flags the same way is done for all the other signals.
>>
>> Cc: Maxime Ripard <maxime@cerno.tech>
> 
> Suggested-by would be nice here :)

Ok, didn't know about this tag

>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
>>   drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>>   2 files changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> index 52598bb0fb0b..30171ccd87e5 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> @@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>>   	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>>   		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>>   
>> -	/*
>> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
>> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
>> -	 * By default TCON works in Negative Edge(Falling Edge),
>> -	 * this is why phase is set to 0 in that case.
>> -	 * Unfortunately there's no way to logically invert dclk through
>> -	 * IO_POL register.
>> -	 * The only acceptable way to work, triple checked with scope,
>> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
>> -	 * for Positive Edge.
>> -	 * On A33 and similar SoCs there would be a 90° phase option,
>> -	 * but it divides also dclk by 2.
>> -	 * Following code is a way to avoid quirks all around TCON
>> -	 * and DOTCLOCK drivers.
>> -	 */
>>   	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
>> -		clk_set_phase(tcon->dclk, 0);
>> -
>> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>> -		clk_set_phase(tcon->dclk, 240);
>> +		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
> 
> I'm not really sure why we need the first patch of this series here?

The idea was to have 2 for testing, 1st one is already applicable, while 
the other must be tested, but I can send only one with no problem.

> That patch only seem to undo what you did in patch 1

No, it doesn't, the 2nd one change the way it achieve the same thing, 
because the 1st swap DCLK phase, while the 2nd uses the IO_POL bit to 
set IO polarity according to bus_flags.

Best Regards
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3213 bytes --]

On Fri, Jan 08, 2021 at 03:34:52PM +0100, Giulio Benetti wrote:
> Hi,
> 
> On 1/8/21 10:23 AM, Maxime Ripard wrote:
> > Hi,
> > 
> > Thanks for those patches
> > 
> > On Thu, Jan 07, 2021 at 03:30:32AM +0100, Giulio Benetti wrote:
> > > From: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > > 
> > > It turned out(Maxime suggestion) that bit 26 of SUN4I_TCON0_IO_POL_REG is
> > > dedicated to invert DCLK polarity and this makes thing really easier than
> > > before. So let's handle DCLK polarity by adding
> > > SUN4I_TCON0_IO_POL_DCLK_POSITIVE as bit 26 and activating according to
> > > bus_flags the same way is done for all the other signals.
> > > 
> > > Cc: Maxime Ripard <maxime@cerno.tech>
> > 
> > Suggested-by would be nice here :)
> 
> Ok, didn't know about this tag
> 
> > > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > > ---
> > >   drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
> > >   drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
> > >   2 files changed, 2 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > index 52598bb0fb0b..30171ccd87e5 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> > >   	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
> > >   		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
> > > -	/*
> > > -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
> > > -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
> > > -	 * By default TCON works in Negative Edge(Falling Edge),
> > > -	 * this is why phase is set to 0 in that case.
> > > -	 * Unfortunately there's no way to logically invert dclk through
> > > -	 * IO_POL register.
> > > -	 * The only acceptable way to work, triple checked with scope,
> > > -	 * is using clock phase set to 0° for Negative Edge and set to 240°
> > > -	 * for Positive Edge.
> > > -	 * On A33 and similar SoCs there would be a 90° phase option,
> > > -	 * but it divides also dclk by 2.
> > > -	 * Following code is a way to avoid quirks all around TCON
> > > -	 * and DOTCLOCK drivers.
> > > -	 */
> > >   	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> > > -		clk_set_phase(tcon->dclk, 0);
> > > -
> > > -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> > > -		clk_set_phase(tcon->dclk, 240);
> > > +		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
> > 
> > I'm not really sure why we need the first patch of this series here?
> 
> The idea was to have 2 for testing, 1st one is already applicable, while the
> other must be tested, but I can send only one with no problem.
> 
> > That patch only seem to undo what you did in patch 1
> 
> No, it doesn't, the 2nd one change the way it achieve the same thing,
> because the 1st swap DCLK phase, while the 2nd uses the IO_POL bit to set IO
> polarity according to bus_flags.

It makes sense for testing, but I'm not sure we want to carry it into
the history. Can you squash them both into the same patch?

Thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling

[Giulio Benetti]

On 1/11/21 6:20 PM, Maxime Ripard wrote:
> On Fri, Jan 08, 2021 at 03:34:52PM +0100, Giulio Benetti wrote:
>> Hi,
>>
>> On 1/8/21 10:23 AM, Maxime Ripard wrote:
>>> Hi,
>>>
>>> Thanks for those patches
>>>
>>> On Thu, Jan 07, 2021 at 03:30:32AM +0100, Giulio Benetti wrote:
>>>> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
>>>>
>>>> It turned out(Maxime suggestion) that bit 26 of SUN4I_TCON0_IO_POL_REG is
>>>> dedicated to invert DCLK polarity and this makes thing really easier than
>>>> before. So let's handle DCLK polarity by adding
>>>> SUN4I_TCON0_IO_POL_DCLK_POSITIVE as bit 26 and activating according to
>>>> bus_flags the same way is done for all the other signals.
>>>>
>>>> Cc: Maxime Ripard <maxime@cerno.tech>
>>>
>>> Suggested-by would be nice here :)
>>
>> Ok, didn't know about this tag
>>
>>>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>>>> ---
>>>>    drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
>>>>    drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>>>>    2 files changed, 2 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>> index 52598bb0fb0b..30171ccd87e5 100644
>>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>> @@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>>>>    	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>>>>    		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>>>> -	/*
>>>> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
>>>> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
>>>> -	 * By default TCON works in Negative Edge(Falling Edge),
>>>> -	 * this is why phase is set to 0 in that case.
>>>> -	 * Unfortunately there's no way to logically invert dclk through
>>>> -	 * IO_POL register.
>>>> -	 * The only acceptable way to work, triple checked with scope,
>>>> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
>>>> -	 * for Positive Edge.
>>>> -	 * On A33 and similar SoCs there would be a 90° phase option,
>>>> -	 * but it divides also dclk by 2.
>>>> -	 * Following code is a way to avoid quirks all around TCON
>>>> -	 * and DOTCLOCK drivers.
>>>> -	 */
>>>>    	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
>>>> -		clk_set_phase(tcon->dclk, 0);
>>>> -
>>>> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>>>> -		clk_set_phase(tcon->dclk, 240);
>>>> +		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
>>>
>>> I'm not really sure why we need the first patch of this series here?
>>
>> The idea was to have 2 for testing, 1st one is already applicable, while the
>> other must be tested, but I can send only one with no problem.
>>
>>> That patch only seem to undo what you did in patch 1
>>
>> No, it doesn't, the 2nd one change the way it achieve the same thing,
>> because the 1st swap DCLK phase, while the 2nd uses the IO_POL bit to set IO
>> polarity according to bus_flags.
> 
> It makes sense for testing, but I'm not sure we want to carry it into
> the history. Can you squash them both into the same patch?
Sure, I'm going to send V3 then.

Thank you
Best regards
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti@micronovasrl.com>

During commit 88bc4178568b ("drm: Use new
DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
macros have been changed to avoid ambiguity but just because of this
ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
_SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
instead of swapping phase values, let's adopt an easier approach Maxime
suggested:
It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
invert DCLK polarity and this makes things really easier than before. So
let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_POSITIVE as
bit 26 and activating according to bus_flags the same way it is done for
all the other signals polarity.

Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
Suggested-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
 2 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index eaaf5d70e352..30171ccd87e5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
 		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
 
-	/*
-	 * On A20 and similar SoCs, the only way to achieve Positive Edge
-	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
-	 * By default TCON works in Negative Edge(Falling Edge),
-	 * this is why phase is set to 0 in that case.
-	 * Unfortunately there's no way to logically invert dclk through
-	 * IO_POL register.
-	 * The only acceptable way to work, triple checked with scope,
-	 * is using clock phase set to 0° for Negative Edge and set to 240°
-	 * for Positive Edge.
-	 * On A33 and similar SoCs there would be a 90° phase option,
-	 * but it divides also dclk by 2.
-	 * Following code is a way to avoid quirks all around TCON
-	 * and DOTCLOCK drivers.
-	 */
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
-		clk_set_phase(tcon->dclk, 240);
-
-	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
-		clk_set_phase(tcon->dclk, 0);
+		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
 			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index cfbf4e6c1679..0ce71d10a31b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -113,6 +113,7 @@
 #define SUN4I_TCON0_IO_POL_REG			0x88
 #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
 #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
+#define SUN4I_TCON0_IO_POL_DCLK_POSITIVE		BIT(26)
 #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
 #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
 
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2884 bytes --]

Hi,

On Mon, Jan 11, 2021 at 06:46:16PM +0100, Giulio Benetti wrote:
> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
> 
> During commit 88bc4178568b ("drm: Use new
> DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
> macros have been changed to avoid ambiguity but just because of this
> ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
> _SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
> instead of swapping phase values, let's adopt an easier approach Maxime
> suggested:
> It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
> invert DCLK polarity and this makes things really easier than before. So
> let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_POSITIVE as
> bit 26 and activating according to bus_flags the same way it is done for
> all the other signals polarity.
> 
> Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
> Suggested-by: Maxime Ripard <maxime@cerno.tech>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>  2 files changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index eaaf5d70e352..30171ccd87e5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>  	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>  		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>  
> -	/*
> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
> -	 * By default TCON works in Negative Edge(Falling Edge),
> -	 * this is why phase is set to 0 in that case.
> -	 * Unfortunately there's no way to logically invert dclk through
> -	 * IO_POL register.
> -	 * The only acceptable way to work, triple checked with scope,
> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
> -	 * for Positive Edge.
> -	 * On A33 and similar SoCs there would be a 90° phase option,
> -	 * but it divides also dclk by 2.
> -	 * Following code is a way to avoid quirks all around TCON
> -	 * and DOTCLOCK drivers.
> -	 */
>  	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> -		clk_set_phase(tcon->dclk, 240);
> -
> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> -		clk_set_phase(tcon->dclk, 0);
> +		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
>  
>  	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
>  			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |

You need to add SUN4I_TCON0_IO_POL_DCLK_POSITIVE to the mask you're
going to change here too

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4] drm/sun4i: tcon: fix inverted DCLK polarity

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti@micronovasrl.com>

During commit 88bc4178568b ("drm: Use new
DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
macros have been changed to avoid ambiguity but just because of this
ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
_SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
instead of swapping phase values, let's adopt an easier approach Maxime
suggested:
It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
invert DCLK polarity and this makes things really easier than before. So
let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_POSITIVE as
bit 26 and activating according to bus_flags the same way it is done for
all the other signals polarity.

Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
Suggested-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
V2->V3:
- squash 2 patches into 1
V3->V4:
- add SUN4I_TCON0_IO_POL_DCLK_POSITIVE to regmap_update_bits()
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 21 ++-------------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index eaaf5d70e352..6e454d316852 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -569,30 +569,13 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
 		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
 
-	/*
-	 * On A20 and similar SoCs, the only way to achieve Positive Edge
-	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
-	 * By default TCON works in Negative Edge(Falling Edge),
-	 * this is why phase is set to 0 in that case.
-	 * Unfortunately there's no way to logically invert dclk through
-	 * IO_POL register.
-	 * The only acceptable way to work, triple checked with scope,
-	 * is using clock phase set to 0° for Negative Edge and set to 240°
-	 * for Positive Edge.
-	 * On A33 and similar SoCs there would be a 90° phase option,
-	 * but it divides also dclk by 2.
-	 * Following code is a way to avoid quirks all around TCON
-	 * and DOTCLOCK drivers.
-	 */
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
-		clk_set_phase(tcon->dclk, 240);
-
-	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
-		clk_set_phase(tcon->dclk, 0);
+		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
 			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
 			   SUN4I_TCON0_IO_POL_VSYNC_POSITIVE |
+			   SUN4I_TCON0_IO_POL_DCLK_POSITIVE |
 			   SUN4I_TCON0_IO_POL_DE_NEGATIVE,
 			   val);
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index cfbf4e6c1679..0ce71d10a31b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -113,6 +113,7 @@
 #define SUN4I_TCON0_IO_POL_REG			0x88
 #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
 #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
+#define SUN4I_TCON0_IO_POL_DCLK_POSITIVE		BIT(26)
 #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
 #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
 
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti@micronovasrl.com>

During commit 88bc4178568b ("drm: Use new
DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
macros have been changed to avoid ambiguity but just because of this
ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
_SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
instead of swapping phase values, let's adopt an easier approach Maxime
suggested:
It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
invert DCLK polarity and this makes things really easier than before. So
let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
as bit 26 and activating according to bus_flags the same way it is done
for all the other signals polarity.

Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
Suggested-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
V2->V3:
- squash 2 patches into 1
V3->V4:
- add SUN4I_TCON0_IO_POL_DCLK_POSITIVE to regmap_update_bits()
V4->V5:
polarity is still wrong so:
- let's use SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE macro
  instead of _DCLK_POSITIVE(that would make sense only in realtion with DCLK)
- invert condition using _NEGEDGE instead of _POSEDGE and then matching with
  register bit SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
- correct commit log according to V4->V5 changes
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 21 ++-------------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index eaaf5d70e352..c172ccfff7e5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -569,30 +569,13 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
 		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
 
-	/*
-	 * On A20 and similar SoCs, the only way to achieve Positive Edge
-	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
-	 * By default TCON works in Negative Edge(Falling Edge),
-	 * this is why phase is set to 0 in that case.
-	 * Unfortunately there's no way to logically invert dclk through
-	 * IO_POL register.
-	 * The only acceptable way to work, triple checked with scope,
-	 * is using clock phase set to 0° for Negative Edge and set to 240°
-	 * for Positive Edge.
-	 * On A33 and similar SoCs there would be a 90° phase option,
-	 * but it divides also dclk by 2.
-	 * Following code is a way to avoid quirks all around TCON
-	 * and DOTCLOCK drivers.
-	 */
-	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
-		clk_set_phase(tcon->dclk, 240);
-
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
-		clk_set_phase(tcon->dclk, 0);
+		val |= SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE;
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
 			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
 			   SUN4I_TCON0_IO_POL_VSYNC_POSITIVE |
+			   SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGDGE |
 			   SUN4I_TCON0_IO_POL_DE_NEGATIVE,
 			   val);
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index cfbf4e6c1679..c5ac1b02482c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -113,6 +113,7 @@
 #define SUN4I_TCON0_IO_POL_REG			0x88
 #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
 #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
+#define SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE		BIT(26)
 #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
 #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
 
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5906 bytes --]

Hi Giulio,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sunxi/sunxi/for-next]
[also build test ERROR on drm-exynos/exynos-drm-next drm-intel/for-linux-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip v5.11-rc3 next-20210113]
[cannot apply to mripard/sunxi/for-next drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Giulio-Benetti/drm-sun4i-tcon-fix-inverted-DCLK-polarity/20210114-001027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
config: arm64-randconfig-r005-20210113 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/acbc865cfd67e02cb2e4a37dfd56598eeae38287
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Giulio-Benetti/drm-sun4i-tcon-fix-inverted-DCLK-polarity/20210114-001027
        git checkout acbc865cfd67e02cb2e4a37dfd56598eeae38287
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/sun4i/sun4i_tcon.c: In function 'sun4i_tcon0_mode_set_rgb':
>> drivers/gpu/drm/sun4i/sun4i_tcon.c:578:7: error: 'SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGDGE' undeclared (first use in this function); did you mean 'SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE'?
     578 |       SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGDGE |
         |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |       SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
   drivers/gpu/drm/sun4i/sun4i_tcon.c:578:7: note: each undeclared identifier is reported only once for each function it appears in


vim +578 drivers/gpu/drm/sun4i/sun4i_tcon.c

   502	
   503	static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
   504					     const struct drm_encoder *encoder,
   505					     const struct drm_display_mode *mode)
   506	{
   507		struct drm_connector *connector = sun4i_tcon_get_connector(encoder);
   508		const struct drm_display_info *info = &connector->display_info;
   509		unsigned int bp, hsync, vsync;
   510		u8 clk_delay;
   511		u32 val = 0;
   512	
   513		WARN_ON(!tcon->quirks->has_channel_0);
   514	
   515		tcon->dclk_min_div = tcon->quirks->dclk_min_div;
   516		tcon->dclk_max_div = 127;
   517		sun4i_tcon0_mode_set_common(tcon, mode);
   518	
   519		/* Set dithering if needed */
   520		sun4i_tcon0_mode_set_dithering(tcon, connector);
   521	
   522		/* Adjust clock delay */
   523		clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
   524		regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
   525				   SUN4I_TCON0_CTL_CLK_DELAY_MASK,
   526				   SUN4I_TCON0_CTL_CLK_DELAY(clk_delay));
   527	
   528		/*
   529		 * This is called a backporch in the register documentation,
   530		 * but it really is the back porch + hsync
   531		 */
   532		bp = mode->crtc_htotal - mode->crtc_hsync_start;
   533		DRM_DEBUG_DRIVER("Setting horizontal total %d, backporch %d\n",
   534				 mode->crtc_htotal, bp);
   535	
   536		/* Set horizontal display timings */
   537		regmap_write(tcon->regs, SUN4I_TCON0_BASIC1_REG,
   538			     SUN4I_TCON0_BASIC1_H_TOTAL(mode->crtc_htotal) |
   539			     SUN4I_TCON0_BASIC1_H_BACKPORCH(bp));
   540	
   541		/*
   542		 * This is called a backporch in the register documentation,
   543		 * but it really is the back porch + hsync
   544		 */
   545		bp = mode->crtc_vtotal - mode->crtc_vsync_start;
   546		DRM_DEBUG_DRIVER("Setting vertical total %d, backporch %d\n",
   547				 mode->crtc_vtotal, bp);
   548	
   549		/* Set vertical display timings */
   550		regmap_write(tcon->regs, SUN4I_TCON0_BASIC2_REG,
   551			     SUN4I_TCON0_BASIC2_V_TOTAL(mode->crtc_vtotal * 2) |
   552			     SUN4I_TCON0_BASIC2_V_BACKPORCH(bp));
   553	
   554		/* Set Hsync and Vsync length */
   555		hsync = mode->crtc_hsync_end - mode->crtc_hsync_start;
   556		vsync = mode->crtc_vsync_end - mode->crtc_vsync_start;
   557		DRM_DEBUG_DRIVER("Setting HSYNC %d, VSYNC %d\n", hsync, vsync);
   558		regmap_write(tcon->regs, SUN4I_TCON0_BASIC3_REG,
   559			     SUN4I_TCON0_BASIC3_V_SYNC(vsync) |
   560			     SUN4I_TCON0_BASIC3_H_SYNC(hsync));
   561	
   562		/* Setup the polarity of the various signals */
   563		if (mode->flags & DRM_MODE_FLAG_PHSYNC)
   564			val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
   565	
   566		if (mode->flags & DRM_MODE_FLAG_PVSYNC)
   567			val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
   568	
   569		if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
   570			val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
   571	
   572		if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
   573			val |= SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE;
   574	
   575		regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
   576				   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
   577				   SUN4I_TCON0_IO_POL_VSYNC_POSITIVE |
 > 578				   SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGDGE |
   579				   SUN4I_TCON0_IO_POL_DE_NEGATIVE,
   580				   val);
   581	
   582		/* Map output pins to channel 0 */
   583		regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG,
   584				   SUN4I_TCON_GCTL_IOMAP_MASK,
   585				   SUN4I_TCON_GCTL_IOMAP_TCON0);
   586	
   587		/* Enable the output on the pins */
   588		regmap_write(tcon->regs, SUN4I_TCON0_IO_TRI_REG, 0);
   589	}
   590	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36180 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity

[Marjan Pascolo]

Hi Giulio,

You did a typo

Il 13/01/2021 17:05, Giulio Benetti ha scritto:
> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
>
> During commit 88bc4178568b ("drm: Use new
> DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
> macros have been changed to avoid ambiguity but just because of this
> ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
> _SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
> instead of swapping phase values, let's adopt an easier approach Maxime
> suggested:
> It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
> invert DCLK polarity and this makes things really easier than before. So
> let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
> as bit 26 and activating according to bus_flags the same way it is done
> for all the other signals polarity.
>
> Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
> Suggested-by: Maxime Ripard <maxime@cerno.tech>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
> V2->V3:
> - squash 2 patches into 1
> V3->V4:
> - add SUN4I_TCON0_IO_POL_DCLK_POSITIVE to regmap_update_bits()
> V4->V5:
> polarity is still wrong so:
> - let's use SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE macro
>    instead of _DCLK_POSITIVE(that would make sense only in realtion with DCLK)
> - invert condition using _NEGEDGE instead of _POSEDGE and then matching with
>    register bit SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
> - correct commit log according to V4->V5 changes
> ---
>   drivers/gpu/drm/sun4i/sun4i_tcon.c | 21 ++-------------------
>   drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>   2 files changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index eaaf5d70e352..c172ccfff7e5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -569,30 +569,13 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>   	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>   		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>   
> -	/*
> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
> -	 * By default TCON works in Negative Edge(Falling Edge),
> -	 * this is why phase is set to 0 in that case.
> -	 * Unfortunately there's no way to logically invert dclk through
> -	 * IO_POL register.
> -	 * The only acceptable way to work, triple checked with scope,
> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
> -	 * for Positive Edge.
> -	 * On A33 and similar SoCs there would be a 90° phase option,
> -	 * but it divides also dclk by 2.
> -	 * Following code is a way to avoid quirks all around TCON
> -	 * and DOTCLOCK drivers.
> -	 */
> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> -		clk_set_phase(tcon->dclk, 240);
> -
>   	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> -		clk_set_phase(tcon->dclk, 0);
> +		val |= SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE;
>   
>   	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
>   			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
>   			   SUN4I_TCON0_IO_POL_VSYNC_POSITIVE |
Here Below you missed an 'E'
> +			   SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGDGE |
>   			   SUN4I_TCON0_IO_POL_DE_NEGATIVE,
>   			   val);
>   
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> index cfbf4e6c1679..c5ac1b02482c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> @@ -113,6 +113,7 @@
>   #define SUN4I_TCON0_IO_POL_REG			0x88
>   #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
>   #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
> +#define SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE		BIT(26)
>   #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
>   #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
>   
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity

[Giulio Benetti]

Hi Marjan,

On 1/14/21 8:58 AM, Marjan Pascolo wrote:
> Hi Giulio,
> 
> You did a typo
> 
> Il 13/01/2021 17:05, Giulio Benetti ha scritto:
>> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
>>
>> During commit 88bc4178568b ("drm: Use new
>> DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
>> macros have been changed to avoid ambiguity but just because of this
>> ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
>> _SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
>> instead of swapping phase values, let's adopt an easier approach Maxime
>> suggested:
>> It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
>> invert DCLK polarity and this makes things really easier than before. So
>> let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
>> as bit 26 and activating according to bus_flags the same way it is done
>> for all the other signals polarity.
>>
>> Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
>> Suggested-by: Maxime Ripard <maxime@cerno.tech>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>> V2->V3:
>> - squash 2 patches into 1
>> V3->V4:
>> - add SUN4I_TCON0_IO_POL_DCLK_POSITIVE to regmap_update_bits()
>> V4->V5:
>> polarity is still wrong so:
>> - let's use SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE macro
>>     instead of _DCLK_POSITIVE(that would make sense only in realtion with DCLK)
>> - invert condition using _NEGEDGE instead of _POSEDGE and then matching with
>>     register bit SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
>> - correct commit log according to V4->V5 changes
>> ---
>>    drivers/gpu/drm/sun4i/sun4i_tcon.c | 21 ++-------------------
>>    drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>>    2 files changed, 3 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> index eaaf5d70e352..c172ccfff7e5 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> @@ -569,30 +569,13 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>>    	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>>    		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>>    
>> -	/*
>> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
>> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
>> -	 * By default TCON works in Negative Edge(Falling Edge),
>> -	 * this is why phase is set to 0 in that case.
>> -	 * Unfortunately there's no way to logically invert dclk through
>> -	 * IO_POL register.
>> -	 * The only acceptable way to work, triple checked with scope,
>> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
>> -	 * for Positive Edge.
>> -	 * On A33 and similar SoCs there would be a 90° phase option,
>> -	 * but it divides also dclk by 2.
>> -	 * Following code is a way to avoid quirks all around TCON
>> -	 * and DOTCLOCK drivers.
>> -	 */
>> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
>> -		clk_set_phase(tcon->dclk, 240);
>> -
>>    	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>> -		clk_set_phase(tcon->dclk, 0);
>> +		val |= SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE;
>>    
>>    	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
>>    			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
>>    			   SUN4I_TCON0_IO_POL_VSYNC_POSITIVE |
> Here Below you missed an 'E'

yes, thank you, going to send v6.

Best regards
-- 
Giulio Benetti
Benetti Engineering sas

>> +			   SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGDGE |
>>    			   SUN4I_TCON0_IO_POL_DE_NEGATIVE,
>>    			   val);
>>    
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
>> index cfbf4e6c1679..c5ac1b02482c 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
>> @@ -113,6 +113,7 @@
>>    #define SUN4I_TCON0_IO_POL_REG			0x88
>>    #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
>>    #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
>> +#define SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE		BIT(26)
>>    #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
>>    #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
>>    

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti@micronovasrl.com>

During commit 88bc4178568b ("drm: Use new
DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
macros have been changed to avoid ambiguity but just because of this
ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
_SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
instead of swapping phase values, let's adopt an easier approach Maxime
suggested:
It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
invert DCLK polarity and this makes things really easier than before. So
let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
as bit 26 and activating according to bus_flags the same way it is done
for all the other signals polarity.

Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
Suggested-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
V2->V3:
- squash 2 patches into 1
V3->V4:
- add SUN4I_TCON0_IO_POL_DCLK_POSITIVE to regmap_update_bits() as suggested by Maxime
V4->V5:
polarity is still wrong so:
- let's use SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE macro
  instead of _DCLK_POSITIVE(that would make sense only in realtion with DCLK)
- invert condition using _NEGEDGE instead of _POSEDGE and then matching with
  register bit SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
- correct commit log according to V4->V5 changes
V5->V6:
- fix typo in SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE as suggested by Marjan
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 21 ++-------------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index eaaf5d70e352..6b9af4c08cd6 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -569,30 +569,13 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
 		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
 
-	/*
-	 * On A20 and similar SoCs, the only way to achieve Positive Edge
-	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
-	 * By default TCON works in Negative Edge(Falling Edge),
-	 * this is why phase is set to 0 in that case.
-	 * Unfortunately there's no way to logically invert dclk through
-	 * IO_POL register.
-	 * The only acceptable way to work, triple checked with scope,
-	 * is using clock phase set to 0° for Negative Edge and set to 240°
-	 * for Positive Edge.
-	 * On A33 and similar SoCs there would be a 90° phase option,
-	 * but it divides also dclk by 2.
-	 * Following code is a way to avoid quirks all around TCON
-	 * and DOTCLOCK drivers.
-	 */
-	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
-		clk_set_phase(tcon->dclk, 240);
-
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
-		clk_set_phase(tcon->dclk, 0);
+		val |= SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE;
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
 			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
 			   SUN4I_TCON0_IO_POL_VSYNC_POSITIVE |
+			   SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE |
 			   SUN4I_TCON0_IO_POL_DE_NEGATIVE,
 			   val);
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index cfbf4e6c1679..c5ac1b02482c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -113,6 +113,7 @@
 #define SUN4I_TCON0_IO_POL_REG			0x88
 #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
 #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
+#define SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE		BIT(26)
 #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
 #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
 
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity

[Marjan Pascolo]

Hello,

Tested on a13 and works.

Here are results:

with DRIVE_NEGEDGE

https://pasteboard.co/JJAGDAy.jpg

https://pasteboard.co/JJAHDAj.jpg

with DRIVE_POSEDGE

https://pasteboard.co/JJAIbBf.jpg

https://pasteboard.co/JJAIGfo.jpg


Il 14/01/2021 09:17, Giulio Benetti ha scritto:
> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
>
> During commit 88bc4178568b ("drm: Use new
> DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
> macros have been changed to avoid ambiguity but just because of this
> ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
> _SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
> instead of swapping phase values, let's adopt an easier approach Maxime
> suggested:
> It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
> invert DCLK polarity and this makes things really easier than before. So
> let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
> as bit 26 and activating according to bus_flags the same way it is done
> for all the other signals polarity.
>
> Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
> Suggested-by: Maxime Ripard <maxime@cerno.tech>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>

Tested-by: Marjan Pascolo <marjan.pascolo@trexom.it>
[Tested successfully with A13]

> ---
> V2->V3:
> - squash 2 patches into 1
> V3->V4:
> - add SUN4I_TCON0_IO_POL_DCLK_POSITIVE to regmap_update_bits() as suggested by Maxime
> V4->V5:
> polarity is still wrong so:
> - let's use SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE macro
>    instead of _DCLK_POSITIVE(that would make sense only in realtion with DCLK)
> - invert condition using _NEGEDGE instead of _POSEDGE and then matching with
>    register bit SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
> - correct commit log according to V4->V5 changes
> V5->V6:
> - fix typo in SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE as suggested by Marjan
> ---
>   drivers/gpu/drm/sun4i/sun4i_tcon.c | 21 ++-------------------
>   drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>   2 files changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index eaaf5d70e352..6b9af4c08cd6 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -569,30 +569,13 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>   	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>   		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>   
> -	/*
> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
> -	 * By default TCON works in Negative Edge(Falling Edge),
> -	 * this is why phase is set to 0 in that case.
> -	 * Unfortunately there's no way to logically invert dclk through
> -	 * IO_POL register.
> -	 * The only acceptable way to work, triple checked with scope,
> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
> -	 * for Positive Edge.
> -	 * On A33 and similar SoCs there would be a 90° phase option,
> -	 * but it divides also dclk by 2.
> -	 * Following code is a way to avoid quirks all around TCON
> -	 * and DOTCLOCK drivers.
> -	 */
> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> -		clk_set_phase(tcon->dclk, 240);
> -
>   	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> -		clk_set_phase(tcon->dclk, 0);
> +		val |= SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE;
>   
>   	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
>   			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
>   			   SUN4I_TCON0_IO_POL_VSYNC_POSITIVE |
> +			   SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE |
>   			   SUN4I_TCON0_IO_POL_DE_NEGATIVE,
>   			   val);
>   
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> index cfbf4e6c1679..c5ac1b02482c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> @@ -113,6 +113,7 @@
>   #define SUN4I_TCON0_IO_POL_REG			0x88
>   #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
>   #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
> +#define SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE		BIT(26)
>   #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
>   #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
>   
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 4175 bytes --]

On Thu, Jan 14, 2021 at 09:17:32AM +0100, Giulio Benetti wrote:
> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
> 
> During commit 88bc4178568b ("drm: Use new
> DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
> macros have been changed to avoid ambiguity but just because of this
> ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
> _SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
> instead of swapping phase values, let's adopt an easier approach Maxime
> suggested:
> It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
> invert DCLK polarity and this makes things really easier than before. So
> let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
> as bit 26 and activating according to bus_flags the same way it is done
> for all the other signals polarity.
> 
> Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
> Suggested-by: Maxime Ripard <maxime@cerno.tech>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
> V2->V3:
> - squash 2 patches into 1
> V3->V4:
> - add SUN4I_TCON0_IO_POL_DCLK_POSITIVE to regmap_update_bits() as suggested by Maxime
> V4->V5:
> polarity is still wrong so:
> - let's use SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE macro
>   instead of _DCLK_POSITIVE(that would make sense only in realtion with DCLK)
> - invert condition using _NEGEDGE instead of _POSEDGE and then matching with
>   register bit SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE
> - correct commit log according to V4->V5 changes
> V5->V6:
> - fix typo in SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE as suggested by Marjan
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 21 ++-------------------
>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>  2 files changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index eaaf5d70e352..6b9af4c08cd6 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -569,30 +569,13 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>  	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>  		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>  
> -	/*
> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
> -	 * By default TCON works in Negative Edge(Falling Edge),
> -	 * this is why phase is set to 0 in that case.
> -	 * Unfortunately there's no way to logically invert dclk through
> -	 * IO_POL register.
> -	 * The only acceptable way to work, triple checked with scope,
> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
> -	 * for Positive Edge.
> -	 * On A33 and similar SoCs there would be a 90° phase option,
> -	 * but it divides also dclk by 2.
> -	 * Following code is a way to avoid quirks all around TCON
> -	 * and DOTCLOCK drivers.
> -	 */
> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> -		clk_set_phase(tcon->dclk, 240);
> -
>  	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> -		clk_set_phase(tcon->dclk, 0);
> +		val |= SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE;
>  
>  	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
>  			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
>  			   SUN4I_TCON0_IO_POL_VSYNC_POSITIVE |
> +			   SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE |
>  			   SUN4I_TCON0_IO_POL_DE_NEGATIVE,
>  			   val);
>  
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> index cfbf4e6c1679..c5ac1b02482c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> @@ -113,6 +113,7 @@
>  #define SUN4I_TCON0_IO_POL_REG			0x88
>  #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
>  #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
> +#define SUN4I_TCON0_IO_POL_DCLK_DRIVE_NEGEDGE		BIT(26)
>  #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
>  #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
>  
> -- 
> 2.25.1
> 
Applied, thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] pinctrl/sunxi: adding input-debounce-ns property

[Marjan Pascolo]

On Allwinner SoC interrupt debounce can be controlled by two oscillator
(32KHz and 24MHz) and a prescale divider.
Oscillator and prescale divider are set through
device tree property "input-debounce" which have 1uS accuracy.
For acheive nS precision a new device tree poperty is made
named "input-debounce-ns".
"input-debounce-ns" is checked only if "input-debounce"
property is not defined.


Suggested-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Marjan Pascolo <marjan.pascolo@trexom.it>
---
---
  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  |  9 +++++++
  drivers/pinctrl/sunxi/pinctrl-sunxi.c         | 25 ++++++++++++++++---
  2 files changed, 30 insertions(+), 4 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml 
b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
index 5240487dfe50..346776de3a44 100644
--- 
a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
+++ 
b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
@@ -93,6 +93,15 @@ properties:
      minItems: 1
      maxItems: 5

+  input-debounce-ns:
+    description:
+      Debouncing periods in nanoseconds, one period per interrupt
+      bank found in the controller.
+      Only checked if input-debounce is not present
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 5
+
  patternProperties:
    # It's pretty scary, but the basic idea is that:
    #   - One node name can start with either s- or r- for PRCM nodes,
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c 
b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index dc8d39ae045b..869b6d5743ba 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -1335,14 +1335,31 @@ static int sunxi_pinctrl_setup_debounce(struct 
sunxi_pinctrl *pctl,
      struct clk *hosc, *losc;
      u8 div, src;
      int i, ret;
+    /* Keeping for loop below clean */
+    const char* debounce_prop_name;
+    unsigned long debounce_dividend;

      /* Deal with old DTs that didn't have the oscillators */
      if (of_clk_get_parent_count(node) != 3)
          return 0;

+    /*
+     * Distinguish between simple input-debounce
+     * and new input-debounce-ns
+     */
+
      /* If we don't have any setup, bail out */
-    if (!of_find_property(node, "input-debounce", NULL))
-        return 0;
+    if (!of_find_property(node, "input-debounce", NULL)) {
+        if(!of_find_property(node, "input-debounce-ns", NULL)) {
+            return 0;
+        } else {
+            debounce_prop_name="input-debounce-ns";
+            debounce_dividend=NSEC_PER_SEC;
+        }
+    } else {
+        debounce_prop_name="input-debounce";
+        debounce_dividend=USEC_PER_SEC;
+    }

      losc = devm_clk_get(pctl->dev, "losc");
      if (IS_ERR(losc))
@@ -1356,7 +1373,7 @@ static int sunxi_pinctrl_setup_debounce(struct 
sunxi_pinctrl *pctl,
          unsigned long debounce_freq;
          u32 debounce;

-        ret = of_property_read_u32_index(node, "input-debounce",
+        ret = of_property_read_u32_index(node, debounce_prop_name,
                           i, &debounce);
          if (ret)
              return ret;
@@ -1364,7 +1381,7 @@ static int sunxi_pinctrl_setup_debounce(struct 
sunxi_pinctrl *pctl,
          if (!debounce)
              continue;

-        debounce_freq = DIV_ROUND_CLOSEST(USEC_PER_SEC, debounce);
+        debounce_freq = DIV_ROUND_CLOSEST(debounce_dividend, debounce);
          losc_div = sunxi_pinctrl_get_debounce_div(losc,
                                debounce_freq,
                                &losc_diff);
-- 
2.22.0.windows.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3457 bytes --]

Hi,

On Wed, Feb 10, 2021 at 05:22:37PM +0100, Marjan Pascolo wrote:
> On Allwinner SoC interrupt debounce can be controlled by two oscillator
> (32KHz and 24MHz) and a prescale divider.
> Oscillator and prescale divider are set through
> device tree property "input-debounce" which have 1uS accuracy.
> For acheive nS precision a new device tree poperty is made
> named "input-debounce-ns".
> "input-debounce-ns" is checked only if "input-debounce"
> property is not defined.
> 
> Suggested-by: Maxime Ripard <maxime@cerno.tech>
> Signed-off-by: Marjan Pascolo <marjan.pascolo@trexom.it>
> ---
> ---
>  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  |  9 +++++++
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c         | 25 ++++++++++++++++---
>  2 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> index 5240487dfe50..346776de3a44 100644
> ---
> a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> +++
> b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> @@ -93,6 +93,15 @@ properties:
>      minItems: 1
>      maxItems: 5
> 
> +  input-debounce-ns:
> +    description:
> +      Debouncing periods in nanoseconds, one period per interrupt
> +      bank found in the controller.
> +      Only checked if input-debounce is not present
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 5
> +

This should be a separate patch, with the DT maintainers in Cc. 

You should enforce that the properties are mutually exclusive through
the schema too

>  patternProperties:
>    # It's pretty scary, but the basic idea is that:
>    #   - One node name can start with either s- or r- for PRCM nodes,
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index dc8d39ae045b..869b6d5743ba 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -1335,14 +1335,31 @@ static int sunxi_pinctrl_setup_debounce(struct
> sunxi_pinctrl *pctl,
>      struct clk *hosc, *losc;
>      u8 div, src;
>      int i, ret;
> +    /* Keeping for loop below clean */
> +    const char* debounce_prop_name;
> +    unsigned long debounce_dividend;
> 
>      /* Deal with old DTs that didn't have the oscillators */
>      if (of_clk_get_parent_count(node) != 3)
>          return 0;
> 
> +    /*
> +     * Distinguish between simple input-debounce
> +     * and new input-debounce-ns
> +     */
> +

I'm not sure that comment should stay, the code is obvious enough

>      /* If we don't have any setup, bail out */
> -    if (!of_find_property(node, "input-debounce", NULL))
> -        return 0;
> +    if (!of_find_property(node, "input-debounce", NULL)) {
> +        if(!of_find_property(node, "input-debounce-ns", NULL)) {
> +            return 0;
> +        } else {
> +            debounce_prop_name="input-debounce-ns";
> +            debounce_dividend=NSEC_PER_SEC;
> +        }
> +    } else {
> +        debounce_prop_name="input-debounce";
> +        debounce_dividend=USEC_PER_SEC;
> +    }

This doesn't follow the kernel coding style, make sure to run
scripts/checkpatch.pl on your patches before sending them.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property

[Marjan Pascolo]

Hi Maxime,

Il 17/02/2021 12:03, Maxime Ripard ha scritto:
> Hi,
>
> On Wed, Feb 10, 2021 at 05:22:37PM +0100, Marjan Pascolo wrote:
>> On Allwinner SoC interrupt debounce can be controlled by two oscillator
>> (32KHz and 24MHz) and a prescale divider.
>> Oscillator and prescale divider are set through
>> device tree property "input-debounce" which have 1uS accuracy.
>> For acheive nS precision a new device tree poperty is made
>> named "input-debounce-ns".
>> "input-debounce-ns" is checked only if "input-debounce"
>> property is not defined.
>>
>> Suggested-by: Maxime Ripard <maxime@cerno.tech>
>> Signed-off-by: Marjan Pascolo <marjan.pascolo@trexom.it>
>> ---
>> ---
>>   .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  |  9 +++++++
>>   drivers/pinctrl/sunxi/pinctrl-sunxi.c         | 25 ++++++++++++++++---
>>   2 files changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
>> b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
>> index 5240487dfe50..346776de3a44 100644
>> ---
>> a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
>> +++
>> b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
>> @@ -93,6 +93,15 @@ properties:
>>       minItems: 1
>>       maxItems: 5
>>
>> +  input-debounce-ns:
>> +    description:
>> +      Debouncing periods in nanoseconds, one period per interrupt
>> +      bank found in the controller.
>> +      Only checked if input-debounce is not present
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    minItems: 1
>> +    maxItems: 5
>> +
> This should be a separate patch, with the DT maintainers in Cc.
>
> You should enforce that the properties are mutually exclusive through
> the schema too

I'm sorry, I've ignored documentaion about /Documentation.

I see that some additional YAML operator (like oneOf) are used.

oneOf should fit the schema, but I can't understand if oneOf's options 
must be a literal value, or if could also be a node.

Otherwise I'll use if ..then..else.


>
>>   patternProperties:
>>     # It's pretty scary, but the basic idea is that:
>>     #   - One node name can start with either s- or r- for PRCM nodes,
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> index dc8d39ae045b..869b6d5743ba 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> @@ -1335,14 +1335,31 @@ static int sunxi_pinctrl_setup_debounce(struct
>> sunxi_pinctrl *pctl,
>>       struct clk *hosc, *losc;
>>       u8 div, src;
>>       int i, ret;
>> +    /* Keeping for loop below clean */
>> +    const char* debounce_prop_name;
>> +    unsigned long debounce_dividend;
>>
>>       /* Deal with old DTs that didn't have the oscillators */
>>       if (of_clk_get_parent_count(node) != 3)
>>           return 0;
>>
>> +    /*
>> +     * Distinguish between simple input-debounce
>> +     * and new input-debounce-ns
>> +     */
>> +
> I'm not sure that comment should stay, the code is obvious enough
>
>>       /* If we don't have any setup, bail out */
>> -    if (!of_find_property(node, "input-debounce", NULL))
>> -        return 0;
>> +    if (!of_find_property(node, "input-debounce", NULL)) {
>> +        if(!of_find_property(node, "input-debounce-ns", NULL)) {
>> +            return 0;
>> +        } else {
>> +            debounce_prop_name="input-debounce-ns";
>> +            debounce_dividend=NSEC_PER_SEC;
>> +        }
>> +    } else {
>> +        debounce_prop_name="input-debounce";
>> +        debounce_dividend=USEC_PER_SEC;
>> +    }
> This doesn't follow the kernel coding style, make sure to run
> scripts/checkpatch.pl on your patches before sending them.
Spaces between operators, right. After /Documentation submission I'll 
resubmit this one
>
> Maxime
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2455 bytes --]

On Fri, Feb 26, 2021 at 01:53:00PM +0100, Marjan Pascolo wrote:
> Hi Maxime,
> 
> Il 17/02/2021 12:03, Maxime Ripard ha scritto:
> > Hi,
> > 
> > On Wed, Feb 10, 2021 at 05:22:37PM +0100, Marjan Pascolo wrote:
> > > On Allwinner SoC interrupt debounce can be controlled by two oscillator
> > > (32KHz and 24MHz) and a prescale divider.
> > > Oscillator and prescale divider are set through
> > > device tree property "input-debounce" which have 1uS accuracy.
> > > For acheive nS precision a new device tree poperty is made
> > > named "input-debounce-ns".
> > > "input-debounce-ns" is checked only if "input-debounce"
> > > property is not defined.
> > > 
> > > Suggested-by: Maxime Ripard <maxime@cerno.tech>
> > > Signed-off-by: Marjan Pascolo <marjan.pascolo@trexom.it>
> > > ---
> > > ---
> > >   .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  |  9 +++++++
> > >   drivers/pinctrl/sunxi/pinctrl-sunxi.c         | 25 ++++++++++++++++---
> > >   2 files changed, 30 insertions(+), 4 deletions(-)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > index 5240487dfe50..346776de3a44 100644
> > > ---
> > > a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > +++
> > > b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > @@ -93,6 +93,15 @@ properties:
> > >       minItems: 1
> > >       maxItems: 5
> > > 
> > > +  input-debounce-ns:
> > > +    description:
> > > +      Debouncing periods in nanoseconds, one period per interrupt
> > > +      bank found in the controller.
> > > +      Only checked if input-debounce is not present
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +    minItems: 1
> > > +    maxItems: 5
> > > +
> > This should be a separate patch, with the DT maintainers in Cc.
> > 
> > You should enforce that the properties are mutually exclusive through
> > the schema too
> 
> I'm sorry, I've ignored documentaion about /Documentation.
> 
> I see that some additional YAML operator (like oneOf) are used.
> 
> oneOf should fit the schema, but I can't understand if oneOf's options must
> be a literal value, or if could also be a node.
> 
> Otherwise I'll use if ..then..else.

dependencies is what you're looking for, not oneOf or if

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity

[Giulio Benetti]

On 1/13/21 10:42 AM, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jan 11, 2021 at 06:46:16PM +0100, Giulio Benetti wrote:
>> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
>>
>> During commit 88bc4178568b ("drm: Use new
>> DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags") DRM_BUS_FLAG_*
>> macros have been changed to avoid ambiguity but just because of this
>> ambiguity previous DRM_BUS_FLAG_PIXDATA_(POS/NEG)EDGE were used meaning
>> _SAMPLE_ not _DRIVE_. This leads to DLCK inversion and need to fix but
>> instead of swapping phase values, let's adopt an easier approach Maxime
>> suggested:
>> It turned out that bit 26 of SUN4I_TCON0_IO_POL_REG is dedicated to
>> invert DCLK polarity and this makes things really easier than before. So
>> let's handle DCLK polarity by adding SUN4I_TCON0_IO_POL_DCLK_POSITIVE as
>> bit 26 and activating according to bus_flags the same way it is done for
>> all the other signals polarity.
>>
>> Fixes: 88bc4178568b ("drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags")
>> Suggested-by: Maxime Ripard <maxime@cerno.tech>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
>>   drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
>>   2 files changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> index eaaf5d70e352..30171ccd87e5 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> @@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>>   	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
>>   		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
>>   
>> -	/*
>> -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
>> -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
>> -	 * By default TCON works in Negative Edge(Falling Edge),
>> -	 * this is why phase is set to 0 in that case.
>> -	 * Unfortunately there's no way to logically invert dclk through
>> -	 * IO_POL register.
>> -	 * The only acceptable way to work, triple checked with scope,
>> -	 * is using clock phase set to 0° for Negative Edge and set to 240°
>> -	 * for Positive Edge.
>> -	 * On A33 and similar SoCs there would be a 90° phase option,
>> -	 * but it divides also dclk by 2.
>> -	 * Following code is a way to avoid quirks all around TCON
>> -	 * and DOTCLOCK drivers.
>> -	 */
>>   	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
>> -		clk_set_phase(tcon->dclk, 240);
>> -
>> -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>> -		clk_set_phase(tcon->dclk, 0);
>> +		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
>>   
>>   	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
>>   			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
> 
> You need to add SUN4I_TCON0_IO_POL_DCLK_POSITIVE to the mask you're
> going to change here too

Ah, lost it during squash, I send a v4.

Thank you
Best regards
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/sun4i: tcon: improve DCLK polarity handling

[Giulio Benetti]

It turned out(Maxime suggestion) that bit 26 of SUN4I_TCON0_IO_POL_REG is
dedicated to invert DCLK polarity and this makes thing really easier than
before. So let's handle DCLK polarity by adding
SUN4I_TCON0_IO_POL_DCLK_POSITIVE as bit 26 and activating according to
bus_flags the same way is done for all the other signals.

Cc: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
 2 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 52598bb0fb0b..30171ccd87e5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
 		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
 
-	/*
-	 * On A20 and similar SoCs, the only way to achieve Positive Edge
-	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
-	 * By default TCON works in Negative Edge(Falling Edge),
-	 * this is why phase is set to 0 in that case.
-	 * Unfortunately there's no way to logically invert dclk through
-	 * IO_POL register.
-	 * The only acceptable way to work, triple checked with scope,
-	 * is using clock phase set to 0° for Negative Edge and set to 240°
-	 * for Positive Edge.
-	 * On A33 and similar SoCs there would be a 90° phase option,
-	 * but it divides also dclk by 2.
-	 * Following code is a way to avoid quirks all around TCON
-	 * and DOTCLOCK drivers.
-	 */
 	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
-		clk_set_phase(tcon->dclk, 0);
-
-	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
-		clk_set_phase(tcon->dclk, 240);
+		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
 			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index cfbf4e6c1679..0ce71d10a31b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -113,6 +113,7 @@
 #define SUN4I_TCON0_IO_POL_REG			0x88
 #define SUN4I_TCON0_IO_POL_DCLK_PHASE(phase)		((phase & 3) << 28)
 #define SUN4I_TCON0_IO_POL_DE_NEGATIVE			BIT(27)
+#define SUN4I_TCON0_IO_POL_DCLK_POSITIVE		BIT(26)
 #define SUN4I_TCON0_IO_POL_HSYNC_POSITIVE		BIT(25)
 #define SUN4I_TCON0_IO_POL_VSYNC_POSITIVE		BIT(24)
 
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel