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 <[email protected]> > --- > 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:
