* [PATCH] video: fbdev: mxsfb: fix pixelclock polarity
@ 2016-01-26 1:29 Stefan Agner
2016-05-06 20:29 ` Stefan Agner
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Agner @ 2016-01-26 1:29 UTC (permalink / raw)
To: plagnioj, shawnguo
Cc: s.hauer, Ying.Liu, linux-fbdev, linux-kernel, LW, fabio.estevam,
max.krummenacher, Stefan Agner
The PIXDATA flags of the display_flags enum are controller centric,
e.g. NEGEDGE means the controller shall drive the data signals on
pixelclocks negative edge. However, the drivers flag is display
centric: Sample the data on negative (falling) edge.
Therefore, change the if statement to check for the POSEDGE flag
(which is typically not set):
Drive on positive edge => sample on negative edge
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Hi all
Shawn, I would like to have at least your Ack on this before merge.
It seems that this has been wrong since the driver is able to use
the timings from the device tree, introduced with 669406534b4a
("video: mxsfb: get display timings from device tree").
Not sure how many device trees actually specify the wrong pixel clock
polarity due to that. At least the initial flag convertion from the
old platform data structures done with 0d9f8217db15 ("ARM: mxs: move
display timing configurations into device tree") seems to be affected
and would need to be changed accordingly...
Not sure how we should handle this, maybe just invert all
pixelclk-active properties where the mxsfb driver is in use...?
--
Stefan
drivers/video/fbdev/mxsfb.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
index 4e6608c..38898a9 100644
--- a/drivers/video/fbdev/mxsfb.c
+++ b/drivers/video/fbdev/mxsfb.c
@@ -150,7 +150,7 @@
#define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT (1 << 6)
-#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negtive edge sampling */
+#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negative edge sampling */
enum mxsfb_devtype {
MXSFB_V3,
@@ -788,7 +788,16 @@ static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
host->sync |= MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
- if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+
+ /*
+ * The PIXDATA flags of the display_flags enum are controller
+ * centric, e.g. NEGEDGE means drive data on negative edge.
+ * However, the drivers flag is display centric: Sample the
+ * data on negative (falling) edge. Therefore, check for the
+ * POSEDGE flag:
+ * drive on positive edge => sample on negative edge
+ */
+ if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
host->sync |= MXSFB_SYNC_DOTCLK_FALLING_ACT;
put_display_node:
--
2.7.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] video: fbdev: mxsfb: fix pixelclock polarity
2016-01-26 1:29 [PATCH] video: fbdev: mxsfb: fix pixelclock polarity Stefan Agner
@ 2016-05-06 20:29 ` Stefan Agner
[not found] ` <CGME20171017135621epcas1p39142851c7bc56478165c887ba14ee585@epcas1p3.samsung.com>
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Agner @ 2016-05-06 20:29 UTC (permalink / raw)
To: plagnioj, shawnguo
Cc: s.hauer, Ying.Liu, linux-fbdev, linux-kernel, LW, fabio.estevam,
max.krummenacher
Hi Shawn,
On 2016-01-25 17:29, Stefan Agner wrote:
> The PIXDATA flags of the display_flags enum are controller centric,
> e.g. NEGEDGE means the controller shall drive the data signals on
> pixelclocks negative edge. However, the drivers flag is display
> centric: Sample the data on negative (falling) edge.
>
> Therefore, change the if statement to check for the POSEDGE flag
> (which is typically not set):
> Drive on positive edge => sample on negative edge
Any comment on that patch?
--
Stefan
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> Hi all
>
> Shawn, I would like to have at least your Ack on this before merge.
>
> It seems that this has been wrong since the driver is able to use
> the timings from the device tree, introduced with 669406534b4a
> ("video: mxsfb: get display timings from device tree").
>
> Not sure how many device trees actually specify the wrong pixel clock
> polarity due to that. At least the initial flag convertion from the
> old platform data structures done with 0d9f8217db15 ("ARM: mxs: move
> display timing configurations into device tree") seems to be affected
> and would need to be changed accordingly...
>
> Not sure how we should handle this, maybe just invert all
> pixelclk-active properties where the mxsfb driver is in use...?
>
> --
> Stefan
>
> drivers/video/fbdev/mxsfb.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
> index 4e6608c..38898a9 100644
> --- a/drivers/video/fbdev/mxsfb.c
> +++ b/drivers/video/fbdev/mxsfb.c
> @@ -150,7 +150,7 @@
> #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24
> bit width */
>
> #define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT (1 << 6)
> -#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negtive edge sampling */
> +#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negative edge sampling */
>
> enum mxsfb_devtype {
> MXSFB_V3,
> @@ -788,7 +788,16 @@ static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
>
> if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> host->sync |= MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
> - if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> +
> + /*
> + * The PIXDATA flags of the display_flags enum are controller
> + * centric, e.g. NEGEDGE means drive data on negative edge.
> + * However, the drivers flag is display centric: Sample the
> + * data on negative (falling) edge. Therefore, check for the
> + * POSEDGE flag:
> + * drive on positive edge => sample on negative edge
> + */
> + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> host->sync |= MXSFB_SYNC_DOTCLK_FALLING_ACT;
>
> put_display_node:
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] video: fbdev: mxsfb: fix pixelclock polarity
[not found] ` <CGME20171017135621epcas1p39142851c7bc56478165c887ba14ee585@epcas1p3.samsung.com>
@ 2017-10-17 13:56 ` Bartlomiej Zolnierkiewicz
2017-10-17 16:01 ` Stefan Agner
2017-10-18 2:05 ` Shawn Guo
0 siblings, 2 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2017-10-17 13:56 UTC (permalink / raw)
To: Stefan Agner, shawnguo
Cc: plagnioj, s.hauer, Ying.Liu, linux-fbdev, linux-kernel, LW,
fabio.estevam, max.krummenacher, mauro.salvini
Hi,
Stefan/Shawn:
Should I merge this patch? It seems to be still needed:
https://www.spinics.net/lists/linux-fbdev/msg23112.html
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
On Friday, May 06, 2016 01:29:59 PM Stefan Agner wrote:
> Hi Shawn,
>
> On 2016-01-25 17:29, Stefan Agner wrote:
> > The PIXDATA flags of the display_flags enum are controller centric,
> > e.g. NEGEDGE means the controller shall drive the data signals on
> > pixelclocks negative edge. However, the drivers flag is display
> > centric: Sample the data on negative (falling) edge.
> >
> > Therefore, change the if statement to check for the POSEDGE flag
> > (which is typically not set):
> > Drive on positive edge => sample on negative edge
>
> Any comment on that patch?
>
> --
> Stefan
>
>
> >
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > ---
> > Hi all
> >
> > Shawn, I would like to have at least your Ack on this before merge.
> >
> > It seems that this has been wrong since the driver is able to use
> > the timings from the device tree, introduced with 669406534b4a
> > ("video: mxsfb: get display timings from device tree").
> >
> > Not sure how many device trees actually specify the wrong pixel clock
> > polarity due to that. At least the initial flag convertion from the
> > old platform data structures done with 0d9f8217db15 ("ARM: mxs: move
> > display timing configurations into device tree") seems to be affected
> > and would need to be changed accordingly...
> >
> > Not sure how we should handle this, maybe just invert all
> > pixelclk-active properties where the mxsfb driver is in use...?
> >
> > --
> > Stefan
> >
> > drivers/video/fbdev/mxsfb.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
> > index 4e6608c..38898a9 100644
> > --- a/drivers/video/fbdev/mxsfb.c
> > +++ b/drivers/video/fbdev/mxsfb.c
> > @@ -150,7 +150,7 @@
> > #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24
> > bit width */
> >
> > #define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT (1 << 6)
> > -#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negtive edge sampling */
> > +#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negative edge sampling */
> >
> > enum mxsfb_devtype {
> > MXSFB_V3,
> > @@ -788,7 +788,16 @@ static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
> >
> > if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> > host->sync |= MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
> > - if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > +
> > + /*
> > + * The PIXDATA flags of the display_flags enum are controller
> > + * centric, e.g. NEGEDGE means drive data on negative edge.
> > + * However, the drivers flag is display centric: Sample the
> > + * data on negative (falling) edge. Therefore, check for the
> > + * POSEDGE flag:
> > + * drive on positive edge => sample on negative edge
> > + */
> > + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > host->sync |= MXSFB_SYNC_DOTCLK_FALLING_ACT;
> >
> > put_display_node:
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] video: fbdev: mxsfb: fix pixelclock polarity
2017-10-17 13:56 ` Bartlomiej Zolnierkiewicz
@ 2017-10-17 16:01 ` Stefan Agner
[not found] ` <CGME20171109133407epcas1p23f7ebf05f0d20e00ef56cf64425bdb60@epcas1p2.samsung.com>
2017-10-18 2:05 ` Shawn Guo
1 sibling, 1 reply; 6+ messages in thread
From: Stefan Agner @ 2017-10-17 16:01 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: shawnguo, plagnioj, s.hauer, Ying.Liu, linux-fbdev, linux-kernel,
LW, fabio.estevam, max.krummenacher, mauro.salvini
On 2017-10-17 15:56, Bartlomiej Zolnierkiewicz wrote:
> Hi,
>
> Stefan/Shawn:
>
> Should I merge this patch? It seems to be still needed:
>
> https://www.spinics.net/lists/linux-fbdev/msg23112.html
In my opinion yes, it fixes an issue...
FWIW, I normally use the DRM driver when using mainline which does not
suffer from this issue, so I did not mind much that this fix never made
it...
--
Stefan
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
> On Friday, May 06, 2016 01:29:59 PM Stefan Agner wrote:
>> Hi Shawn,
>>
>> On 2016-01-25 17:29, Stefan Agner wrote:
>> > The PIXDATA flags of the display_flags enum are controller centric,
>> > e.g. NEGEDGE means the controller shall drive the data signals on
>> > pixelclocks negative edge. However, the drivers flag is display
>> > centric: Sample the data on negative (falling) edge.
>> >
>> > Therefore, change the if statement to check for the POSEDGE flag
>> > (which is typically not set):
>> > Drive on positive edge => sample on negative edge
>>
>> Any comment on that patch?
>>
>> --
>> Stefan
>>
>>
>> >
>> > Signed-off-by: Stefan Agner <stefan@agner.ch>
>> > ---
>> > Hi all
>> >
>> > Shawn, I would like to have at least your Ack on this before merge.
>> >
>> > It seems that this has been wrong since the driver is able to use
>> > the timings from the device tree, introduced with 669406534b4a
>> > ("video: mxsfb: get display timings from device tree").
>> >
>> > Not sure how many device trees actually specify the wrong pixel clock
>> > polarity due to that. At least the initial flag convertion from the
>> > old platform data structures done with 0d9f8217db15 ("ARM: mxs: move
>> > display timing configurations into device tree") seems to be affected
>> > and would need to be changed accordingly...
>> >
>> > Not sure how we should handle this, maybe just invert all
>> > pixelclk-active properties where the mxsfb driver is in use...?
>> >
>> > --
>> > Stefan
>> >
>> > drivers/video/fbdev/mxsfb.c | 13 +++++++++++--
>> > 1 file changed, 11 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
>> > index 4e6608c..38898a9 100644
>> > --- a/drivers/video/fbdev/mxsfb.c
>> > +++ b/drivers/video/fbdev/mxsfb.c
>> > @@ -150,7 +150,7 @@
>> > #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24
>> > bit width */
>> >
>> > #define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT (1 << 6)
>> > -#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negtive edge sampling */
>> > +#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negative edge sampling */
>> >
>> > enum mxsfb_devtype {
>> > MXSFB_V3,
>> > @@ -788,7 +788,16 @@ static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
>> >
>> > if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
>> > host->sync |= MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
>> > - if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
>> > +
>> > + /*
>> > + * The PIXDATA flags of the display_flags enum are controller
>> > + * centric, e.g. NEGEDGE means drive data on negative edge.
>> > + * However, the drivers flag is display centric: Sample the
>> > + * data on negative (falling) edge. Therefore, check for the
>> > + * POSEDGE flag:
>> > + * drive on positive edge => sample on negative edge
>> > + */
>> > + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
>> > host->sync |= MXSFB_SYNC_DOTCLK_FALLING_ACT;
>> >
>> > put_display_node:
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] video: fbdev: mxsfb: fix pixelclock polarity
2017-10-17 13:56 ` Bartlomiej Zolnierkiewicz
2017-10-17 16:01 ` Stefan Agner
@ 2017-10-18 2:05 ` Shawn Guo
1 sibling, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2017-10-18 2:05 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Stefan Agner, plagnioj, s.hauer, Ying.Liu, linux-fbdev,
linux-kernel, LW, fabio.estevam, max.krummenacher, mauro.salvini
On Tue, Oct 17, 2017 at 03:56:19PM +0200, Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> Stefan/Shawn:
>
> Should I merge this patch? It seems to be still needed:
Acked-by: Shawn Guo <shawnguo@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] video: fbdev: mxsfb: fix pixelclock polarity
[not found] ` <CGME20171109133407epcas1p23f7ebf05f0d20e00ef56cf64425bdb60@epcas1p2.samsung.com>
@ 2017-11-09 13:34 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2017-11-09 13:34 UTC (permalink / raw)
To: Stefan Agner
Cc: shawnguo, plagnioj, s.hauer, Ying.Liu, linux-fbdev, linux-kernel,
LW, fabio.estevam, max.krummenacher, mauro.salvini
On Tuesday, October 17, 2017 06:01:58 PM Stefan Agner wrote:
> On 2017-10-17 15:56, Bartlomiej Zolnierkiewicz wrote:
> > Hi,
> >
> > Stefan/Shawn:
> >
> > Should I merge this patch? It seems to be still needed:
> >
> > https://www.spinics.net/lists/linux-fbdev/msg23112.html
>
> In my opinion yes, it fixes an issue...
>
> FWIW, I normally use the DRM driver when using mainline which does not
> suffer from this issue, so I did not mind much that this fix never made
> it...
I queued your patch (with Shawn's ACK) for 4.15, thanks!
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> --
> Stefan
>
> >
> > Best regards,
> > --
> > Bartlomiej Zolnierkiewicz
> > Samsung R&D Institute Poland
> > Samsung Electronics
> >
> > On Friday, May 06, 2016 01:29:59 PM Stefan Agner wrote:
> >> Hi Shawn,
> >>
> >> On 2016-01-25 17:29, Stefan Agner wrote:
> >> > The PIXDATA flags of the display_flags enum are controller centric,
> >> > e.g. NEGEDGE means the controller shall drive the data signals on
> >> > pixelclocks negative edge. However, the drivers flag is display
> >> > centric: Sample the data on negative (falling) edge.
> >> >
> >> > Therefore, change the if statement to check for the POSEDGE flag
> >> > (which is typically not set):
> >> > Drive on positive edge => sample on negative edge
> >>
> >> Any comment on that patch?
> >>
> >> --
> >> Stefan
> >>
> >>
> >> >
> >> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> > ---
> >> > Hi all
> >> >
> >> > Shawn, I would like to have at least your Ack on this before merge.
> >> >
> >> > It seems that this has been wrong since the driver is able to use
> >> > the timings from the device tree, introduced with 669406534b4a
> >> > ("video: mxsfb: get display timings from device tree").
> >> >
> >> > Not sure how many device trees actually specify the wrong pixel clock
> >> > polarity due to that. At least the initial flag convertion from the
> >> > old platform data structures done with 0d9f8217db15 ("ARM: mxs: move
> >> > display timing configurations into device tree") seems to be affected
> >> > and would need to be changed accordingly...
> >> >
> >> > Not sure how we should handle this, maybe just invert all
> >> > pixelclk-active properties where the mxsfb driver is in use...?
> >> >
> >> > --
> >> > Stefan
> >> >
> >> > drivers/video/fbdev/mxsfb.c | 13 +++++++++++--
> >> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
> >> > index 4e6608c..38898a9 100644
> >> > --- a/drivers/video/fbdev/mxsfb.c
> >> > +++ b/drivers/video/fbdev/mxsfb.c
> >> > @@ -150,7 +150,7 @@
> >> > #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24
> >> > bit width */
> >> >
> >> > #define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT (1 << 6)
> >> > -#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negtive edge sampling */
> >> > +#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negative edge sampling */
> >> >
> >> > enum mxsfb_devtype {
> >> > MXSFB_V3,
> >> > @@ -788,7 +788,16 @@ static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
> >> >
> >> > if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> >> > host->sync |= MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
> >> > - if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> >> > +
> >> > + /*
> >> > + * The PIXDATA flags of the display_flags enum are controller
> >> > + * centric, e.g. NEGEDGE means drive data on negative edge.
> >> > + * However, the drivers flag is display centric: Sample the
> >> > + * data on negative (falling) edge. Therefore, check for the
> >> > + * POSEDGE flag:
> >> > + * drive on positive edge => sample on negative edge
> >> > + */
> >> > + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> >> > host->sync |= MXSFB_SYNC_DOTCLK_FALLING_ACT;
> >> >
> >> > put_display_node:
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-09 13:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-26 1:29 [PATCH] video: fbdev: mxsfb: fix pixelclock polarity Stefan Agner
2016-05-06 20:29 ` Stefan Agner
[not found] ` <CGME20171017135621epcas1p39142851c7bc56478165c887ba14ee585@epcas1p3.samsung.com>
2017-10-17 13:56 ` Bartlomiej Zolnierkiewicz
2017-10-17 16:01 ` Stefan Agner
[not found] ` <CGME20171109133407epcas1p23f7ebf05f0d20e00ef56cf64425bdb60@epcas1p2.samsung.com>
2017-11-09 13:34 ` Bartlomiej Zolnierkiewicz
2017-10-18 2:05 ` Shawn Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).