* Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling
@ 2021-01-11 17:20 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-01-11 17:20 UTC (permalink / raw)
To: Giulio Benetti
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, treding, Giulio Benetti, Marjan Pascolo
[-- 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling
@ 2021-01-11 17:20 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-01-11 17:20 UTC (permalink / raw)
To: Giulio Benetti
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, daniel, treding, Giulio Benetti,
Marjan Pascolo
[-- 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: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling
2021-01-11 17:20 ` Maxime Ripard
(?)
@ 2021-01-11 17:37 ` Giulio Benetti
-1 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-11 17:37 UTC (permalink / raw)
To: Maxime Ripard
Cc: Marjan Pascolo, wens, daniel, airlied, treding, Jernej Skrabec,
dri-devel, linux-arm-kernel, linux-kernel, 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling
@ 2021-01-11 17:37 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-11 17:37 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, treding, Giulio Benetti, Marjan Pascolo
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling
@ 2021-01-11 17:37 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-11 17:37 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, daniel, treding, Giulio Benetti,
Marjan Pascolo
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-11 17:20 ` Maxime Ripard
(?)
@ 2021-01-11 17:46 ` Giulio Benetti
-1 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-11 17:46 UTC (permalink / raw)
To: Maxime Ripard
Cc: Marjan Pascolo, wens, daniel, airlied, treding, Jernej Skrabec,
dri-devel, linux-kernel, linux-arm-kernel, 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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-11 17:46 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-11 17:46 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, treding, Giulio Benetti, Marjan Pascolo
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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-11 17:46 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-11 17:46 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, daniel, treding, Giulio Benetti,
Marjan Pascolo
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 83+ messages in thread* Re: [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-11 17:46 ` Giulio Benetti
(?)
@ 2021-01-13 9:42 ` Maxime Ripard
-1 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-01-13 9:42 UTC (permalink / raw)
To: Giulio Benetti
Cc: Marjan Pascolo, wens, daniel, airlied, treding, Jernej Skrabec,
dri-devel, linux-kernel, linux-arm-kernel, Giulio Benetti
[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-13 9:42 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-01-13 9:42 UTC (permalink / raw)
To: Giulio Benetti
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, treding, Giulio Benetti, Marjan Pascolo
[-- 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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-13 9:42 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-01-13 9:42 UTC (permalink / raw)
To: Giulio Benetti
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, daniel, treding, Giulio Benetti,
Marjan Pascolo
[-- 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: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread* [PATCH v4] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-13 9:42 ` Maxime Ripard
(?)
@ 2021-01-13 10:47 ` Giulio Benetti
-1 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-13 10:47 UTC (permalink / raw)
To: Maxime Ripard
Cc: Marjan Pascolo, wens, daniel, airlied, treding, Jernej Skrabec,
dri-devel, linux-arm-kernel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH v4] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-13 10:47 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-13 10:47 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, treding, Giulio Benetti, Marjan Pascolo
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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH v4] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-13 10:47 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-13 10:47 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, daniel, treding, Giulio Benetti,
Marjan Pascolo
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-13 10:47 ` Giulio Benetti
(?)
@ 2021-01-13 16:05 ` Giulio Benetti
-1 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-13 16:05 UTC (permalink / raw)
To: Maxime Ripard
Cc: Marjan Pascolo, wens, daniel, airlied, treding, Jernej Skrabec,
dri-devel, linux-kernel, linux-arm-kernel, 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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-13 16:05 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-13 16:05 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, treding, Giulio Benetti, Marjan Pascolo
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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-13 16:05 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-13 16:05 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, daniel, treding, Giulio Benetti,
Marjan Pascolo
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 83+ messages in thread* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-13 16:05 ` Giulio Benetti
(?)
(?)
@ 2021-01-14 1:32 ` kernel test robot
-1 siblings, 0 replies; 83+ messages in thread
From: kernel test robot @ 2021-01-14 1:32 UTC (permalink / raw)
To: Giulio Benetti, Maxime Ripard
Cc: kbuild-all, Marjan Pascolo, wens, daniel, airlied, treding,
Jernej Skrabec, dri-devel, linux-kernel, linux-arm-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 1:32 ` kernel test robot
0 siblings, 0 replies; 83+ messages in thread
From: kernel test robot @ 2021-01-14 1:32 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6041 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(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36180 bytes --]
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 1:32 ` kernel test robot
0 siblings, 0 replies; 83+ messages in thread
From: kernel test robot @ 2021-01-14 1:32 UTC (permalink / raw)
To: Giulio Benetti, Maxime Ripard
Cc: Jernej Skrabec, kbuild-all, airlied, linux-kernel, dri-devel,
wens, linux-arm-kernel, treding, Marjan Pascolo
[-- 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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 1:32 ` kernel test robot
0 siblings, 0 replies; 83+ messages in thread
From: kernel test robot @ 2021-01-14 1:32 UTC (permalink / raw)
To: Giulio Benetti, Maxime Ripard
Cc: Jernej Skrabec, kbuild-all, airlied, linux-kernel, dri-devel,
wens, linux-arm-kernel, daniel, treding, Marjan Pascolo
[-- 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: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-13 16:05 ` Giulio Benetti
(?)
@ 2021-01-14 7:58 ` Marjan Pascolo
-1 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-01-14 7:58 UTC (permalink / raw)
To: Giulio Benetti, Maxime Ripard
Cc: wens, daniel, airlied, treding, Jernej Skrabec, dri-devel,
linux-kernel, linux-arm-kernel, Giulio Benetti
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)
>
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 7:58 ` Marjan Pascolo
0 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-01-14 7:58 UTC (permalink / raw)
To: Giulio Benetti, Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens, treding,
Giulio Benetti, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 7:58 ` Marjan Pascolo
0 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-01-14 7:58 UTC (permalink / raw)
To: Giulio Benetti, Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens, daniel,
treding, Giulio Benetti, linux-arm-kernel
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)
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-14 7:58 ` Marjan Pascolo
(?)
@ 2021-01-14 8:12 ` Giulio Benetti
-1 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-14 8:12 UTC (permalink / raw)
To: Marjan Pascolo, Maxime Ripard
Cc: wens, daniel, airlied, treding, Jernej Skrabec, dri-devel,
linux-kernel, linux-arm-kernel, 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)
>>
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 8:12 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-14 8:12 UTC (permalink / raw)
To: Marjan Pascolo, Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens, treding,
Giulio Benetti, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v5] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 8:12 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-14 8:12 UTC (permalink / raw)
To: Marjan Pascolo, Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens, daniel,
treding, Giulio Benetti, linux-arm-kernel
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)
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-14 7:58 ` Marjan Pascolo
(?)
@ 2021-01-14 8:17 ` Giulio Benetti
-1 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-14 8:17 UTC (permalink / raw)
To: Maxime Ripard
Cc: Marjan Pascolo, wens, daniel, airlied, treding, Jernej Skrabec,
dri-devel, linux-arm-kernel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 8:17 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-14 8:17 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, treding, Giulio Benetti, Marjan Pascolo
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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 8:17 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-14 8:17 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, daniel, treding, Giulio Benetti,
Marjan Pascolo
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 83+ messages in thread* Re: [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-14 8:17 ` Giulio Benetti
(?)
@ 2021-01-14 11:40 ` Marjan Pascolo
-1 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-01-14 11:40 UTC (permalink / raw)
To: Giulio Benetti, Maxime Ripard
Cc: wens, daniel, airlied, treding, Jernej Skrabec, dri-devel,
linux-arm-kernel, linux-kernel, Giulio Benetti
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)
>
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 11:40 ` Marjan Pascolo
0 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-01-14 11:40 UTC (permalink / raw)
To: Giulio Benetti, Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens, treding,
Giulio Benetti, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 11:40 ` Marjan Pascolo
0 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-01-14 11:40 UTC (permalink / raw)
To: Giulio Benetti, Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens, daniel,
treding, Giulio Benetti, linux-arm-kernel
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)
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-14 8:17 ` Giulio Benetti
(?)
@ 2021-01-14 11:42 ` Maxime Ripard
-1 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-01-14 11:42 UTC (permalink / raw)
To: Giulio Benetti
Cc: Marjan Pascolo, wens, daniel, airlied, treding, Jernej Skrabec,
dri-devel, linux-arm-kernel, linux-kernel, Giulio Benetti
[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 11:42 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-01-14 11:42 UTC (permalink / raw)
To: Giulio Benetti
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, treding, Giulio Benetti, Marjan Pascolo
[-- 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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v6] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-14 11:42 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-01-14 11:42 UTC (permalink / raw)
To: Giulio Benetti
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, daniel, treding, Giulio Benetti,
Marjan Pascolo
[-- 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: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread* [PATCH] pinctrl/sunxi: adding input-debounce-ns property
2021-01-14 11:42 ` Maxime Ripard
(?)
@ 2021-02-10 16:22 ` Marjan Pascolo
-1 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-02-10 16:22 UTC (permalink / raw)
To: Maxime Ripard, wens, Jernej Skrabec
Cc: dri-devel, linux-arm-kernel, linux-kernel
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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH] pinctrl/sunxi: adding input-debounce-ns property
@ 2021-02-10 16:22 ` Marjan Pascolo
0 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-02-10 16:22 UTC (permalink / raw)
To: Maxime Ripard, wens, Jernej Skrabec
Cc: linux-arm-kernel, dri-devel, linux-kernel
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
^ permalink raw reply related [flat|nested] 83+ messages in thread* [PATCH] pinctrl/sunxi: adding input-debounce-ns property
@ 2021-02-10 16:22 ` Marjan Pascolo
0 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-02-10 16:22 UTC (permalink / raw)
To: Maxime Ripard, wens, Jernej Skrabec
Cc: linux-arm-kernel, dri-devel, linux-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 83+ messages in thread* Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property
2021-02-10 16:22 ` Marjan Pascolo
(?)
@ 2021-02-17 11:03 ` Maxime Ripard
-1 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-02-17 11:03 UTC (permalink / raw)
To: Marjan Pascolo
Cc: wens, Jernej Skrabec, dri-devel, linux-arm-kernel, linux-kernel
[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property
@ 2021-02-17 11:03 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-02-17 11:03 UTC (permalink / raw)
To: Marjan Pascolo
Cc: linux-arm-kernel, wens, Jernej Skrabec, dri-devel, linux-kernel
[-- 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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property
@ 2021-02-17 11:03 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-02-17 11:03 UTC (permalink / raw)
To: Marjan Pascolo
Cc: linux-arm-kernel, wens, Jernej Skrabec, dri-devel, linux-kernel
[-- 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: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property
2021-02-17 11:03 ` Maxime Ripard
(?)
@ 2021-02-26 12:53 ` Marjan Pascolo
-1 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-02-26 12:53 UTC (permalink / raw)
To: Maxime Ripard
Cc: wens, Jernej Skrabec, dri-devel, linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property
@ 2021-02-26 12:53 ` Marjan Pascolo
0 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-02-26 12:53 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-arm-kernel, wens, Jernej Skrabec, dri-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property
@ 2021-02-26 12:53 ` Marjan Pascolo
0 siblings, 0 replies; 83+ messages in thread
From: Marjan Pascolo @ 2021-02-26 12:53 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-arm-kernel, wens, Jernej Skrabec, dri-devel, linux-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property
2021-02-26 12:53 ` Marjan Pascolo
(?)
@ 2021-03-04 15:51 ` Maxime Ripard
-1 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-03-04 15:51 UTC (permalink / raw)
To: Marjan Pascolo
Cc: wens, Jernej Skrabec, dri-devel, linux-arm-kernel, linux-kernel
[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property
@ 2021-03-04 15:51 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-03-04 15:51 UTC (permalink / raw)
To: Marjan Pascolo
Cc: linux-arm-kernel, wens, Jernej Skrabec, dri-devel, linux-kernel
[-- 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property
@ 2021-03-04 15:51 ` Maxime Ripard
0 siblings, 0 replies; 83+ messages in thread
From: Maxime Ripard @ 2021-03-04 15:51 UTC (permalink / raw)
To: Marjan Pascolo
Cc: wens, Jernej Skrabec, dri-devel, linux-arm-kernel, linux-kernel
[-- 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: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity
2021-01-13 9:42 ` Maxime Ripard
(?)
@ 2021-01-13 10:48 ` Giulio Benetti
-1 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-13 10:48 UTC (permalink / raw)
To: Maxime Ripard
Cc: Marjan Pascolo, wens, daniel, airlied, treding, Jernej Skrabec,
dri-devel, linux-kernel, linux-arm-kernel, 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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-13 10:48 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-13 10:48 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, treding, Giulio Benetti, Marjan Pascolo
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
^ permalink raw reply [flat|nested] 83+ messages in thread* Re: [PATCH v3] drm/sun4i: tcon: fix inverted DCLK polarity
@ 2021-01-13 10:48 ` Giulio Benetti
0 siblings, 0 replies; 83+ messages in thread
From: Giulio Benetti @ 2021-01-13 10:48 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jernej Skrabec, airlied, linux-kernel, dri-devel, wens,
linux-arm-kernel, daniel, treding, Giulio Benetti,
Marjan Pascolo
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 83+ messages in thread
